Re: [Intel-gfx] [PATCH 04/21] drm/i915: skylake scaler structure definitions
On Wed, Mar 18, 2015 at 09:00:20AM +0100, Daniel Vetter wrote: On Tue, Mar 17, 2015 at 09:20:11PM +, Konduru, Chandra wrote: -Original Message- From: Roper, Matthew D Sent: Tuesday, March 17, 2015 9:13 AM To: Konduru, Chandra Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander Subject: Re: [PATCH 04/21] drm/i915: skylake scaler structure definitions On Sat, Mar 14, 2015 at 10:55:29PM -0700, Chandra Konduru wrote: skylake scaler structure definitions. scalers live in crtc_state as they are pipe resources. They can be used either as plane scaler or panel fitter. scaler assigned to either plane (for plane scaling) or crtc (for panel fitting) is saved in scaler_id in plane_state or crtc_state respectively. scaler_id is used instead of scaler pointer in plane or crtc state to avoid updating scaler pointer everytime a new crtc_state is created. Signed-off-by: Chandra Konduru chandra.kond...@intel.com --- drivers/gpu/drm/i915/intel_drv.h | 99 ++ 1 file changed, 99 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 3f7d05e..d9a3b64 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -256,6 +256,35 @@ struct intel_plane_state { * enable/disable the primary plane */ bool hides_primary; + + /* +* scaler_id +*= -1 : not using a scaler +*= 0 : using a scalers +* +* plane requiring a scaler: +* - During check_plane, its bit is set in +* crtc_state-scaler_state.scaler_users by calling helper function +* update_scaler_users. +* - scaler_id indicates the scaler it got assigned. +* +* plane doesn't require a scaler: +* - this can happen when scaling is no more required or plane simply +* got disabled. +* - During check_plane, corresponding bit is reset in +* crtc_state-scaler_state.scaler_users by calling helper function +* update_scaler_users. +* +* There are two scenarios: +* 1. the freed up scaler is assigned to crtc or some other plane +* In this case, as part of plane programming scaler_id will be set +* to -1 using helper function detach_scalers +* 2. the freed up scaler is not assigned to anyone +* In this case, as part of plane programming scaler registers will +* be reset and scaler_id will also be reset to -1 using the same +* helper function detach_scalers +*/ + int scaler_id; }; struct intel_initial_plane_config { @@ -265,6 +294,74 @@ struct intel_initial_plane_config { u32 base; }; +struct intel_scaler { + int id; + int in_use; + uint32_t mode; + uint32_t filter; + + /* +* Supported scaling ratio is represented as a range in [min max] +* variables. This range covers both up and downscaling +* where scaling ratio = (dst * 100)/src. +* In above range any value: +* 100 represents downscaling coverage +* 100 represents upscaling coverage +*= 100 represents no-scaling (i.e., 1:1) +* e.g., a min value = 50 means - supports upto 50% of original image +* a max value = 200 means - supports upto 200% of original image +* +* if incoming flip requires scaling in the supported [min max] range +* then requested scaling will be performed. +*/ I've only skimmed a little ahead in the series, so I might have missed something, but do we really need to track these on a per-scaler basis? When you use the values here, I think you're always pulling the values from scaler[0] unconditionally from what I saw so duplicating the values for each scaler doesn't really help us. Is it possible to keep just one copy of these min/max values? On SKL all of our scalers are homogeneous, so it doesn't feel like there's too much value to duplicating these values. If we have a future platform with heterogeneous scalers, it seems like we can figure out how to handle that appropriately if/when we get there. I put them per scaler basis for future scalability, but can make them as one copy. It has some cascading effect on other patches so send out this one and other impacted ones. In my experience adding flexibility in the design for
Re: [Intel-gfx] [PATCH 6/7] drm/i915: Boost GPU frequency if we detect outstanding pageflips
On Wednesday 18 March 2015 01:48 PM, Deepak S wrote: On Friday 06 March 2015 08:36 PM, Chris Wilson wrote: If we hit a vblank and see that have a pageflip queue but not yet processed, ensure that the GPU is running at maximum in order to clear the backlog. Pageflips are only queued for the following vblank, if we miss it, there will be a visible stutter. Boosting the GPU frequency doesn't prevent us from missing the target vblank, but it should help the subsequent frames hitting theirs. v2: Reorder vblank vs flip-complete so that we only check for a missed flip after processing the completion events, and avoid spurious boosts. v3: Rename missed_vblank v4: Rebase v5: Cancel the outstanding work in runtime suspend v6: Rebase Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 11 --- drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_pm.c | 34 ++ 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2412002d510b..7735f0a4184c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9818,6 +9818,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe) struct drm_i915_private *dev_priv = dev-dev_private; struct drm_crtc *crtc = dev_priv-pipe_to_crtc_mapping[pipe]; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); +struct intel_unpin_work *work; WARN_ON(!in_irq()); @@ -9825,12 +9826,16 @@ void intel_check_page_flip(struct drm_device *dev, int pipe) return; spin_lock(dev-event_lock); -if (intel_crtc-unpin_work __intel_pageflip_stall_check(dev, crtc)) { +work = intel_crtc-unpin_work; +if (work != NULL __intel_pageflip_stall_check(dev, crtc)) { WARN_ONCE(1, Kicking stuck page flip: queued at %d, now %d\n, - intel_crtc-unpin_work-flip_queued_vblank, - drm_vblank_count(dev, pipe)); + work-flip_queued_vblank, drm_vblank_count(dev, pipe)); page_flip_completed(intel_crtc); +work = NULL; } +if (work != NULL +drm_vblank_count(dev, pipe) - work-flip_queued_vblank 1) +intel_queue_rps_boost_for_request(dev, work-flip_queued_req); spin_unlock(dev-event_lock); } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1e564da2fd38..87e09a58191a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1242,6 +1242,8 @@ void gen6_rps_busy(struct drm_i915_private *dev_priv); void gen6_rps_reset_ei(struct drm_i915_private *dev_priv); void gen6_rps_idle(struct drm_i915_private *dev_priv); void gen6_rps_boost(struct drm_i915_private *dev_priv); +void intel_queue_rps_boost_for_request(struct drm_device *dev, + struct drm_i915_gem_request *rq); void ilk_wm_get_hw_state(struct drm_device *dev); void skl_wm_get_hw_state(struct drm_device *dev); void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv, diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 972333d2211d..120b8af3c60c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6598,6 +6598,40 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val) return val / GT_FREQUENCY_MULTIPLIER; } +struct request_boost { +struct work_struct work; +struct drm_i915_gem_request *rq; +}; + +static void __intel_rps_boost_work(struct work_struct *work) +{ +struct request_boost *boost = container_of(work, struct request_boost, work); + +if (!i915_gem_request_completed(boost-rq, true)) +gen6_rps_boost(to_i915(boost-rq-ring-dev)); + +i915_gem_request_unreference(boost-rq); +kfree(boost); +} + +void intel_queue_rps_boost_for_request(struct drm_device *dev, + struct drm_i915_gem_request *rq) +{ +struct request_boost *boost; + +if (rq == NULL || INTEL_INFO(dev)-gen 6) +return; + +boost = kmalloc(sizeof(*boost), GFP_ATOMIC); +if (boost == NULL) +return; + +INIT_WORK(boost-work, __intel_rps_boost_work); +i915_gem_request_assign(boost-rq, rq); + +queue_work(to_i915(dev)-wq, boost-work); +} + void intel_pm_setup(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; Patch looks fine Reviewed-by: Deepak Sdeepa...@linux.intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: gen4: work around hang during hibernation
On Wed, Mar 18, 2015 at 10:37:16AM +0100, Paul Bolle wrote: Imre Deak schreef op ma 02-03-2015 om 13:04 [+0200]: Bjørn reported that his machine hang during hibernation and eventually bisected the problem to the following commit: commit da2bc1b9db3351addd293e5b82757efe1f77ed1d Author: Imre Deak imre.d...@intel.com Date: Thu Oct 23 19:23:26 2014 +0300 drm/i915: add poweroff_late handler The problem seems to be that after the kernel puts the device into D3 the BIOS still tries to access it, or otherwise assumes that it's in D0. This is clearly bogus, since ACPI mandates that devices are put into D3 by the OSPM if they are not wake-up sources. In the future we want to unify more of the driver's runtime and system suspend paths, for example by skipping all the system suspend/hibernation hooks if the device is runtime suspended already. Accordingly for all other platforms the goal is still to properly power down the device during hibernation. v2: - Another GEN4 Lenovo laptop had the same issue, while platforms from other vendors (including mobile and desktop, GEN4 and non-GEN4) seem to work fine. Based on this apply the workaround on all GEN4 Lenovo platforms. - add code comment about failing platforms (Ville) The outdated ThinkPad X41 that I torture by running rc's showed identical symptoms, also since v3.19-rc1. It uses a gen3 chipset (it has a 915GM, I think, but I keep forgetting details like that). I did everything wrong to get this fixed (1: hope this gets magically fixed; 2: bisect it myself, thinking every now and then that I know better than git bisect which commit to choose; 3: finally grep lkml). So here I am late to the show. Reference: http://lists.freedesktop.org/archives/intel-gfx/2015-February/060633.html Reported-and-bisected-by: Bjørn Mork bj...@mork.no Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 30 +- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 4badb23..ff3662f 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -637,7 +637,7 @@ static int i915_drm_suspend(struct drm_device *dev) return 0; } -static int i915_drm_suspend_late(struct drm_device *drm_dev) +static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation) { struct drm_i915_private *dev_priv = drm_dev-dev_private; int ret; @@ -651,7 +651,17 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev) } pci_disable_device(drm_dev-pdev); - pci_set_power_state(drm_dev-pdev, PCI_D3hot); + /* +* During hibernation on some GEN4 platforms the BIOS may try to access +* the device even though it's already in D3 and hang the machine. So +* leave the device in D0 on those platforms and hope the BIOS will +* power down the device properly. Platforms where this was seen: +* Lenovo Thinkpad X301, X61s +*/ + if (!(hibernation + drm_dev-pdev-subsystem_vendor == PCI_VENDOR_ID_LENOVO + INTEL_INFO(dev_priv)-gen == 4)) + pci_set_power_state(drm_dev-pdev, PCI_D3hot); return 0; } I'll paste a DRAFT patch that fixes this for that X41 at the end of the message. The patch is rather ugly. Should we perhaps try a quirk table or something like that? Paul Bolle 8 Subject: [PATCH] drm/i915: work around hang during hibernation on gen3 too Commit ab3be73fa7b4 (drm/i915: gen4: work around hang during hibernation) was targetted at gen4 platforms shipped by Lenovo. The same problem can also be seen on a Lenovo ThinkPad X41. Expand the test to catch that system too. Sadly, this system still uses IBM's subsystem vendor id. So we end up with a rather unpleasant test. Use the IS_GEN3() and IS_GEN4() macros to lessen the pain a bit. We had another bug report which showed similar problems on something as recent as SNB: https://bugzilla.kernel.org/show_bug.cgi?id=94241 So I guess we really want to make the check 'gen 7'. My IVB X1 Carbon doesn't need this quirk, so hopefully that indicates the Lenovo BIOSen became more sane for gen7+. Not-yet-signed-off-by: Paul Bolle pebo...@tiscali.nl --- drivers/gpu/drm/i915/i915_drv.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index cc6ea53d2b81..3a07164f5860 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -641,11 +641,12 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation) * the device even though it's already in D3 and hang the machine. So * leave the device in D0 on those platforms and hope the BIOS will * power down the
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Agressive downclocking on Baytrail
On Wednesday 18 March 2015 03:18 PM, Daniel Vetter wrote: On Wed, Mar 18, 2015 at 01:42:58PM +0530, Deepak S wrote: I guess your empty reply wasn't intentional? -Daniel Sorry, that was not intentional :) On Friday 06 March 2015 08:36 PM, Chris Wilson wrote: Reuse the same reclocking strategy for Baytail as on its bigger brethren, Sandybridge and Ivybridge. In particular, this makes the device quicker to reclock (both up and down) though the tendency now is to downclock more aggressively to compensate for the RPS boosts. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Deepak S deepa...@linux.intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Rodrigo Vivi rodrigo.v...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Conflicts: drivers/gpu/drm/i915/intel_pm.c --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_irq.c | 4 ++-- drivers/gpu/drm/i915/i915_reg.h | 2 -- drivers/gpu/drm/i915/intel_pm.c | 11 ++- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index efa98c9e5777..bfa6e11f802e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1028,6 +1028,9 @@ struct intel_gen6_power_mgmt { u8 rp0_freq;/* Non-overclocked max frequency. */ u32 cz_freq; + u8 up_threshold; /* Current %busy required to uplock */ + u8 down_threshold; /* Current %busy required to downclock */ + int last_adj; enum { LOW_POWER, BETWEEN, HIGH_POWER } power; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 8892dbdfb629..483079d96957 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1050,7 +1050,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) if (pm_iir GEN6_PM_RP_DOWN_EI_EXPIRED) { if (!vlv_c0_above(dev_priv, dev_priv-rps.down_ei, now, - VLV_RP_DOWN_EI_THRESHOLD)) + dev_priv-rps.down_threshold)) events |= GEN6_PM_RP_DOWN_THRESHOLD; dev_priv-rps.down_ei = now; } @@ -1058,7 +1058,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) if (pm_iir GEN6_PM_RP_UP_EI_EXPIRED) { if (vlv_c0_above(dev_priv, dev_priv-rps.up_ei, now, -VLV_RP_UP_EI_THRESHOLD)) +dev_priv-rps.up_threshold)) events |= GEN6_PM_RP_UP_THRESHOLD; dev_priv-rps.up_ei = now; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ca24a2d4a823..13ec6c2b1fcf 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -663,8 +663,6 @@ enum skl_disp_power_wells { #define FB_FMAX_VMIN_FREQ_LO_MASK 0xf800 #define VLV_CZ_CLOCK_TO_MILLI_SEC 10 -#define VLV_RP_UP_EI_THRESHOLD 90 -#define VLV_RP_DOWN_EI_THRESHOLD 70 /* vlv2 north clock has */ #define CCK_FUSE_REG 0x8 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 55dc406cd195..972333d2211d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3722,10 +3722,12 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) switch (new_power) { case LOW_POWER: /* Upclock if more than 95% busy over 16ms */ + dev_priv-rps.up_threshold = 95; I915_WRITE(GEN6_RP_UP_EI, 12500); I915_WRITE(GEN6_RP_UP_THRESHOLD, 11800); /* Downclock if less than 85% busy over 32ms */ + dev_priv-rps.down_threshold = 85; I915_WRITE(GEN6_RP_DOWN_EI, 25000); I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 21250); @@ -3740,10 +3742,12 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) case BETWEEN: /* Upclock if more than 90% busy over 13ms */ + dev_priv-rps.up_threshold = 90; I915_WRITE(GEN6_RP_UP_EI, 10250); I915_WRITE(GEN6_RP_UP_THRESHOLD, 9225); /* Downclock if less than 75% busy over 32ms */ + dev_priv-rps.down_threshold = 75; I915_WRITE(GEN6_RP_DOWN_EI, 25000); I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 18750); @@ -3758,10 +3762,12 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) case HIGH_POWER: /* Upclock if more than 85% busy over 10ms */ + dev_priv-rps.up_threshold = 85; I915_WRITE(GEN6_RP_UP_EI, 8000); I915_WRITE(GEN6_RP_UP_THRESHOLD,
Re: [Intel-gfx] [PATCH v2 4/7] drm/i915: Agressive downclocking on Baytrail
On Wednesday 18 March 2015 03:18 PM, Chris Wilson wrote: Reuse the same reclocking strategy for Baytail as on its bigger brethren, Sandybridge and Ivybridge. In particular, this makes the device quicker to reclock (both up and down) though the tendency now is to downclock more aggressively to compensate for the RPS boosts. v2: Rebase Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Deepak S deepa...@linux.intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Rodrigo Vivi rodrigo.v...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_irq.c | 4 ++-- drivers/gpu/drm/i915/i915_reg.h | 2 -- drivers/gpu/drm/i915/intel_pm.c | 7 ++- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b156bc30c9c9..afb552c1a4f8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1031,6 +1031,9 @@ struct intel_gen6_power_mgmt { u8 rp0_freq;/* Non-overclocked max frequency. */ u32 cz_freq; + u8 up_threshold; /* Current %busy required to uplock */ + u8 down_threshold; /* Current %busy required to downclock */ + int last_adj; enum { LOW_POWER, BETWEEN, HIGH_POWER } power; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 6d8340d5a111..58af8e239971 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1050,7 +1050,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) if (pm_iir GEN6_PM_RP_DOWN_EI_EXPIRED) { if (!vlv_c0_above(dev_priv, dev_priv-rps.down_ei, now, - VLV_RP_DOWN_EI_THRESHOLD)) + dev_priv-rps.down_threshold)) events |= GEN6_PM_RP_DOWN_THRESHOLD; dev_priv-rps.down_ei = now; } @@ -1058,7 +1058,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) if (pm_iir GEN6_PM_RP_UP_EI_EXPIRED) { if (vlv_c0_above(dev_priv, dev_priv-rps.up_ei, now, -VLV_RP_UP_EI_THRESHOLD)) +dev_priv-rps.up_threshold)) events |= GEN6_PM_RP_UP_THRESHOLD; dev_priv-rps.up_ei = now; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5b84ee686f99..c94c06b21052 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -671,8 +671,6 @@ enum skl_disp_power_wells { #define FB_FMAX_VMIN_FREQ_LO_MASK 0xf800 #define VLV_CZ_CLOCK_TO_MILLI_SEC 10 -#define VLV_RP_UP_EI_THRESHOLD 90 -#define VLV_RP_DOWN_EI_THRESHOLD 70 /* vlv2 north clock has */ #define CCK_FUSE_REG 0x8 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index e18f0fd22cf2..8b16bb3ae09f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3914,6 +3914,8 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) GEN6_RP_DOWN_IDLE_AVG); dev_priv-rps.power = new_power; + dev_priv-rps.up_threshold = threshold_up; + dev_priv-rps.down_threshold = threshold_down; dev_priv-rps.last_adj = 0; } @@ -3985,8 +3987,10 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val) Odd GPU freq value\n)) val = ~1; - if (val != dev_priv-rps.cur_freq) + if (val != dev_priv-rps.cur_freq) { vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val); + gen6_set_rps_thresholds(dev_priv, val); I think gen6_set_rps_thresholds should be under baytrail specific with platform check? + } I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val)); @@ -4035,6 +4039,7 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) GENFREQSTATUS) == 0, 100)) DRM_ERROR(timed out waiting for Punit\n); + gen6_set_rps_thresholds(dev_priv, val); vlv_force_gfx_clock(dev_priv, false); I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val)); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [patch] drm/i915: memory leak in __i915_gem_vma_create()
In the original code then if WARN_ON(i915_is_ggtt(vm) != !!ggtt_view) was true then we leak vma. Presumably that doesn't happen often but static checkers complain and this bug is easy to fix. Fixes: c3bbb6f2825d ('drm/i915: Do not use ggtt_view with (aliasing) PPGTT') Signed-off-by: Dan Carpenter dan.carpen...@oracle.com diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index f1b9ea6..cbf013f 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2340,12 +2340,13 @@ __i915_gem_vma_create(struct drm_i915_gem_object *obj, struct i915_address_space *vm, const struct i915_ggtt_view *ggtt_view) { - struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL); - if (vma == NULL) - return ERR_PTR(-ENOMEM); + struct i915_vma *vma; if (WARN_ON(i915_is_ggtt(vm) != !!ggtt_view)) return ERR_PTR(-EINVAL); + vma = kzalloc(sizeof(*vma), GFP_KERNEL); + if (vma == NULL) + return ERR_PTR(-ENOMEM); INIT_LIST_HEAD(vma-vma_link); INIT_LIST_HEAD(vma-mm_list); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Fix computation of last_adjustment for RPS autotuning
On Wed, Mar 18, 2015 at 12:26:49PM +0530, Deepak S wrote: On Friday 06 March 2015 08:36 PM, Chris Wilson wrote: I think we should modify adj in GEN6_PM_RP_UP_EI_EXPIRED? if not not we might request higher freq since we add adj to new_delay before request freq. Oh yeah. That branch didn't exist when I first wrote the patch, and didn't exist at the end of the series so I never saw it! For this point in the patch series, yes, we do need to set adj=0. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Fix computation of last_adjustment for RPS autotuning
On Wed, Mar 18, 2015 at 12:26:49PM +0530, Deepak S wrote: On Friday 06 March 2015 08:36 PM, Chris Wilson wrote: Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_irq.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9baecb79de8c..1296ce37e435 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1150,21 +1150,20 @@ static void gen6_pm_rps_work(struct work_struct *work) mutex_lock(dev_priv-rps.hw_lock); adj = dev_priv-rps.last_adj; +new_delay = dev_priv-rps.cur_freq; if (pm_iir GEN6_PM_RP_UP_THRESHOLD) { if (adj 0) adj *= 2; -else { -/* CHV needs even encode values */ -adj = IS_CHERRYVIEW(dev_priv-dev) ? 2 : 1; -} -new_delay = dev_priv-rps.cur_freq + adj; - +else /* CHV needs even encode values */ +adj = IS_CHERRYVIEW(dev_priv) ? 2 : 1; /* * For better performance, jump directly * to RPe if we're below it. */ -if (new_delay dev_priv-rps.efficient_freq) +if (new_delay dev_priv-rps.efficient_freq - adj) { new_delay = dev_priv-rps.efficient_freq; +adj = 0; +} } else if (pm_iir GEN6_PM_RP_DOWN_TIMEOUT) { if (dev_priv-rps.cur_freq dev_priv-rps.efficient_freq) new_delay = dev_priv-rps.efficient_freq; @@ -1176,24 +1175,22 @@ static void gen6_pm_rps_work(struct work_struct *work) I think we should modify adj in GEN6_PM_RP_UP_EI_EXPIRED? if not not we might request higher freq since we add adj to new_delay before request freq. The best way to resolve the conflict appears to be just to reorder this patch later after the removal of the vlv specific adj paths -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 35/49] drm/i915/bxt: fix panel fitter setup in crtc disable/enable
On ke, 2015-03-18 at 09:37 +0100, Daniel Vetter wrote: On Tue, Mar 17, 2015 at 04:22:33PM +0200, Imre Deak wrote: On ti, 2015-03-17 at 14:51 +0100, Daniel Vetter wrote: On Tue, Mar 17, 2015 at 11:40:01AM +0200, Imre Deak wrote: From: Jesse Barnes jbar...@virtuousgeek.org Broxton has the same panel fitter registers as Skylake. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ba2d1ae..95ce0a8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4532,7 +4532,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) intel_ddi_enable_pipe_clock(intel_crtc); - if (IS_SKYLAKE(dev)) + if (INTEL_INFO(dev)-gen == 9) Shouldn't we go with gen = 9 here while at it? == 9 looks like the right choice based on bspec. Then maybe add a else if (gen 9) MISSING_CASE(gen); afterwards? Just trying to avoid surprises when enabling skl/bxt+1 ... Yea, makes sense. I'll update the patch. --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 35/49] drm/i915/bxt: fix panel fitter setup in crtc disable/enable
On Tue, Mar 17, 2015 at 04:22:33PM +0200, Imre Deak wrote: On ti, 2015-03-17 at 14:51 +0100, Daniel Vetter wrote: On Tue, Mar 17, 2015 at 11:40:01AM +0200, Imre Deak wrote: From: Jesse Barnes jbar...@virtuousgeek.org Broxton has the same panel fitter registers as Skylake. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ba2d1ae..95ce0a8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4532,7 +4532,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) intel_ddi_enable_pipe_clock(intel_crtc); - if (IS_SKYLAKE(dev)) + if (INTEL_INFO(dev)-gen == 9) Shouldn't we go with gen = 9 here while at it? == 9 looks like the right choice based on bspec. Then maybe add a else if (gen 9) MISSING_CASE(gen); afterwards? Just trying to avoid surprises when enabling skl/bxt+1 ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] topic/drm-misc
Hi Dave, Another drm-misch pull request. Mostly the fbdev sizes deconfusion series from Rob, everything else is small stuff all over. And the large i2c over aux transfers patch, too. Cheers, Daniel The following changes since commit 7eb5f302bbe78b88da8b2008c502c1975e75db05: drm: Check in setcrtc if the primary plane supports the fb pixel format (2015-03-10 09:59:36 +0100) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/topic/drm-misc-2015-03-18 for you to fetch changes up to 522cf91f30cb0102fd5cb6e8979d45b6151cdcfc: drm: check that planes types are correct while initializing CRTC (2015-03-17 14:03:04 +0100) Benjamin Gaignard (1): drm: check that planes types are correct while initializing CRTC Daniel Vetter (1): drm/plane-helper: Fixup mismerge John Hunter (2): drm: Fix some typo mistake of the annotations drm: change connector to tmp_connector Rob Clark (7): drm/fb: document drm_fb_helper_surface_size drm/atomic: minor kerneldoc typo fix drm/cma: use correct fb width/height drm/exynos: use correct fb width/height drm/rockchip: use correct fb width/height drm/fb: small cleanup drm/fb: handle tiled connectors better Scott Wood (1): drm: %pF is only for function pointers Simon Farnsworth (1): drm/dp: Use large transactions for I2C over AUX Ville Syrjälä (2): drm/atomic: Constify a bunch of functions pointer structs drm: Silence sparse warnings drivers/gpu/drm/drm_atomic_helper.c | 50 +- drivers/gpu/drm/drm_bridge.c | 2 +- drivers/gpu/drm/drm_crtc.c| 3 ++ drivers/gpu/drm/drm_dp_helper.c | 76 --- drivers/gpu/drm/drm_drv.c | 2 +- drivers/gpu/drm/drm_fb_cma_helper.c | 2 +- drivers/gpu/drm/drm_fb_helper.c | 48 - drivers/gpu/drm/drm_info.c| 1 + drivers/gpu/drm/drm_ioc32.c | 2 +- drivers/gpu/drm/drm_pci.c | 1 + drivers/gpu/drm/drm_plane_helper.c| 11 ++-- drivers/gpu/drm/drm_vm.c | 1 + drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 5 +- drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 2 +- include/drm/drm_crtc.h| 2 +- include/drm/drm_dp_helper.h | 5 ++ include/drm/drm_fb_helper.h | 19 +++ 17 files changed, 163 insertions(+), 69 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Fix computation of last_adjustment for RPS autotuning
On Wednesday 18 March 2015 02:50 PM, Chris Wilson wrote: On Wed, Mar 18, 2015 at 12:26:49PM +0530, Deepak S wrote: On Friday 06 March 2015 08:36 PM, Chris Wilson wrote: Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_irq.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9baecb79de8c..1296ce37e435 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1150,21 +1150,20 @@ static void gen6_pm_rps_work(struct work_struct *work) mutex_lock(dev_priv-rps.hw_lock); adj = dev_priv-rps.last_adj; + new_delay = dev_priv-rps.cur_freq; if (pm_iir GEN6_PM_RP_UP_THRESHOLD) { if (adj 0) adj *= 2; - else { - /* CHV needs even encode values */ - adj = IS_CHERRYVIEW(dev_priv-dev) ? 2 : 1; - } - new_delay = dev_priv-rps.cur_freq + adj; - + else /* CHV needs even encode values */ + adj = IS_CHERRYVIEW(dev_priv) ? 2 : 1; /* * For better performance, jump directly * to RPe if we're below it. */ - if (new_delay dev_priv-rps.efficient_freq) + if (new_delay dev_priv-rps.efficient_freq - adj) { new_delay = dev_priv-rps.efficient_freq; + adj = 0; + } } else if (pm_iir GEN6_PM_RP_DOWN_TIMEOUT) { if (dev_priv-rps.cur_freq dev_priv-rps.efficient_freq) new_delay = dev_priv-rps.efficient_freq; @@ -1176,24 +1175,22 @@ static void gen6_pm_rps_work(struct work_struct *work) I think we should modify adj in GEN6_PM_RP_UP_EI_EXPIRED? if not not we might request higher freq since we add adj to new_delay before request freq. The best way to resolve the conflict appears to be just to reorder this patch later after the removal of the vlv specific adj paths -Chris Yes, I saw the reorder patch. looks fine. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/7] drm/i915: Use down ei for manual Baytrail RPS calculations
On Friday 06 March 2015 08:36 PM, Chris Wilson wrote: Use both up/down manual ei calcuations for symmetry and greater flexibility for reclocking, instead of faking the down interrupt based on a fixed integer number of up interrupts. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_drv.h | 2 -- drivers/gpu/drm/i915/i915_irq.c | 15 ++- drivers/gpu/drm/i915/i915_reg.h | 1 - drivers/gpu/drm/i915/intel_pm.c | 5 ++--- 4 files changed, 4 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 73292a183492..efa98c9e5777 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1028,8 +1028,6 @@ struct intel_gen6_power_mgmt { u8 rp0_freq;/* Non-overclocked max frequency. */ u32 cz_freq; - u32 ei_interrupt_count; - int last_adj; enum { LOW_POWER, BETWEEN, HIGH_POWER } power; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 4b7b86298b37..8892dbdfb629 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1033,7 +1033,6 @@ void gen6_rps_reset_ei(struct drm_i915_private *dev_priv) { vlv_c0_read(dev_priv, dev_priv-rps.down_ei); dev_priv-rps.up_ei = dev_priv-rps.down_ei; - dev_priv-rps.ei_interrupt_count = 0; } static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) @@ -1041,23 +1040,13 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) struct intel_rps_ei now; u32 events = 0; - if ((pm_iir GEN6_PM_RP_UP_EI_EXPIRED) == 0) + if ((pm_iir (GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED)) == 0) return 0; vlv_c0_read(dev_priv, now); if (now.cz_clock == 0) return 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) { - pm_iir |= GEN6_PM_RP_DOWN_EI_EXPIRED; - dev_priv-rps.ei_interrupt_count = 0; - } - if (pm_iir GEN6_PM_RP_DOWN_EI_EXPIRED) { if (!vlv_c0_above(dev_priv, dev_priv-rps.down_ei, now, @@ -4247,7 +4236,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv) /* Let's track the enabled rps events */ if (IS_VALLEYVIEW(dev_priv) !IS_CHERRYVIEW(dev_priv)) /* WaGsvRC0ResidencyMethod:vlv */ - dev_priv-pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED; + dev_priv-pm_rps_events = GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED; else dev_priv-pm_rps_events = GEN6_PM_RPS_EVENTS; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 324922d8f8a1..ca24a2d4a823 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -665,7 +665,6 @@ enum skl_disp_power_wells { #define VLV_CZ_CLOCK_TO_MILLI_SEC 10 #define VLV_RP_UP_EI_THRESHOLD90 #define VLV_RP_DOWN_EI_THRESHOLD 70 -#define VLV_INT_COUNT_FOR_DOWN_EI 5 /* vlv2 north clock has */ #define CCK_FUSE_REG 0x8 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index b37d634bea99..55dc406cd195 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3784,11 +3784,10 @@ static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_priv, u8 val) u32 mask = 0; if (val dev_priv-rps.min_freq_softlimit) - mask |= GEN6_PM_RP_DOWN_THRESHOLD | GEN6_PM_RP_DOWN_TIMEOUT; + mask |= GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_DOWN_THRESHOLD | GEN6_PM_RP_DOWN_TIMEOUT; if (val dev_priv-rps.max_freq_softlimit) - mask |= GEN6_PM_RP_UP_THRESHOLD; + mask |= GEN6_PM_RP_UP_EI_EXPIRED | GEN6_PM_RP_UP_THRESHOLD; - mask |= dev_priv-pm_rps_events (GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED); mask = dev_priv-pm_rps_events; return gen6_sanitize_rps_pm_mask(dev_priv, ~mask); Patch looks fine Reviewed-by: Deepak Sdeepa...@linux.intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Regression] WARNING: drivers/gpu/drm/i915/i915_gem.c:4525 i915_gem_free_object
On Wed, 18 Mar 2015, Steven Rostedt rost...@goodmis.org wrote: On Tue, 17 Mar 2015 08:53:21 +0100 Daniel Vetter dan...@ffwll.ch wrote: Can you please cherry pick 42a7b088127f (\drm/i915: Make sure the primary plane is enabled before reading out the fb state\) from -next to 4.0-rc to test whether this is indeed the difference in ducttape? I cherry-picked that patch and the warning does go away. Cherry-picked to drm-intel-fixes to queue it to v4.0-rc. Thanks for the report and testing. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/7] drm/i915: Deminish contribution of wait-boosting from clients
On Friday 06 March 2015 08:36 PM, Chris Wilson wrote: With boosting for missed pageflips, we have a much stronger indication of when we need to (temporarily) boost GPU frequency to ensure smooth delivery of frames. So now only allow each client to perform one RPS boost in each period of GPU activity due to stalling on results. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_debugfs.c | 39 + drivers/gpu/drm/i915/i915_drv.h | 9 ++--- drivers/gpu/drm/i915/i915_gem.c | 35 - drivers/gpu/drm/i915/intel_drv.h| 3 ++- drivers/gpu/drm/i915/intel_pm.c | 18 ++--- 5 files changed, 70 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index cc83cc0ff823..9366eaa50dba 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2245,6 +2245,44 @@ static int i915_ppgtt_info(struct seq_file *m, void *data) return 0; } +static int i915_rps_boost_info(struct seq_file *m, void *data) +{ + struct drm_info_node *node = m-private; + struct drm_device *dev = node-minor-dev; + struct drm_i915_private *dev_priv = dev-dev_private; + struct drm_file *file; + int ret; + + ret = mutex_lock_interruptible(dev-struct_mutex); + if (ret) + return ret; + + ret = mutex_lock_interruptible(dev_priv-rps.hw_lock); + if (ret) + goto unlock; + + list_for_each_entry_reverse(file, dev-filelist, lhead) { + struct drm_i915_file_private *file_priv = file-driver_priv; + struct task_struct *task; + + rcu_read_lock(); + task = pid_task(file-pid, PIDTYPE_PID); + seq_printf(m, %s [%d]: %d boosts%s\n, + task ? task-comm : unknown, + task ? task-pid : -1, + file_priv-rps_boosts, + list_empty(file_priv-rps_boost) ? : , active); + rcu_read_unlock(); + } + seq_printf(m, Kernel boosts: %d\n, dev_priv-rps.boosts); + + mutex_unlock(dev_priv-rps.hw_lock); +unlock: + mutex_unlock(dev-struct_mutex); + + return ret; +} + static int i915_llc(struct seq_file *m, void *data) { struct drm_info_node *node = m-private; @@ -4680,6 +4718,7 @@ static const struct drm_info_list i915_debugfs_list[] = { {i915_ddb_info, i915_ddb_info, 0}, {i915_sseu_status, i915_sseu_status, 0}, {i915_drrs_status, i915_drrs_status, 0}, + {i915_rps_boost_info, i915_rps_boost_info, 0}, }; #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index bfa6e11f802e..b207d2cec9dc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1036,6 +1036,8 @@ struct intel_gen6_power_mgmt { bool enabled; struct delayed_work delayed_resume_work; + struct list_head clients; + unsigned boosts; /* manual wa residency calculations */ struct intel_rps_ei up_ei, down_ei; @@ -2137,12 +2139,13 @@ struct drm_i915_file_private { struct { spinlock_t lock; struct list_head request_list; - struct delayed_work idle_work; } mm; struct idr context_idr; - atomic_t rps_wait_boost; - struct intel_engine_cs *bsd_ring; + struct list_head rps_boost; + struct intel_engine_cs *bsd_ring; + + unsigned rps_boosts; }; /* diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index b266b31690e4..a0c35f80836c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1191,14 +1191,6 @@ static bool missed_irq(struct drm_i915_private *dev_priv, return test_bit(ring-id, dev_priv-gpu_error.missed_irq_rings); } -static bool can_wait_boost(struct drm_i915_file_private *file_priv) -{ - if (file_priv == NULL) - return true; - - return !atomic_xchg(file_priv-rps_wait_boost, true); -} - /** * __i915_wait_request - wait until execution of request has finished * @req: duh! @@ -1240,13 +1232,8 @@ int __i915_wait_request(struct drm_i915_gem_request *req, timeout_expire = timeout ? jiffies + nsecs_to_jiffies_timeout((u64)*timeout) : 0; - if (INTEL_INFO(dev)-gen = 6 ring-id == RCS can_wait_boost(file_priv)) { - gen6_rps_boost(dev_priv); - if (file_priv) - mod_delayed_work(dev_priv-wq, -file_priv-mm.idle_work, -msecs_to_jiffies(100)); - } + if (ring-id == RCS INTEL_INFO(dev)-gen = 6) +
Re: [Intel-gfx] [PATCH] drm/i915: Turn on PIN_GLOBAL in i915_gem_object_ggtt_pin
On Tue, Mar 17, 2015 at 03:36:51PM +, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com This makes the interface consistent to old i915_gem_obj_ggtt_pin. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Cc: Joonas Lahtinen joonas.lahti...@linux.intel.com Ok our naming of gem/vma pin/unpin/bind/unbind functions seems to be a rather impressive mess now. We have lots of special-cases with devilishly close names and distinct behaviour. This needs to be cleaned up once rotated and partial views have landed. Queued for -next meanwhile, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Return current vblank value for drmWaitVBlank queries
On Wed, Mar 18, 2015 at 11:53:16AM +0900, Michel Dänzer wrote: drm_vblank_count_and_time() doesn't return the correct sequence number while the vblank interrupt is disabled, does it? It returns the sequence number from the last time vblank_disable_and_save() was called (when the vblank interrupt was disabled). That's why drm_vblank_get() is needed here. Ville enlightened me as well. I thought the value was cooked so that time did not pass whilst the IRQ was disabled. Hopefully, I can impress upon the Intel folks, at least, that enabling/disabling the interrupts just to read the current hw counter is interesting to say the least and sits at the top of the profiles when benchmarking Present. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/7] drm/i915: Improved w/a for rps on Baytrail
Rewrite commit 31685c258e0b0ad6aa486c5ec001382cf8a64212 Author: Deepak S deepa...@linux.intel.com Date: Thu Jul 3 17:33:01 2014 -0400 drm/i915/vlv: WA for Turbo and RC6 to work together. Other than code clarity, the major improvement is to disable the extra interrupts generated when idle. However, the reclocking remains rather slow under the new manual regime, in particular it fails to downclock as quickly as desired. The second major improvement is that for certain workloads, like games, we need to combine render+media activity counters as the work of displaying the frame is split across the engines and both need to be taken into account when deciding the global GPU frequency as memory cycles are shared. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Deepak S deepa...@linux.intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Rodrigo Vivi rodrigo.v...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Reviewed-by: Deepak Sdeepa...@linux.intel.com --- drivers/gpu/drm/i915/i915_irq.c | 155 +-- drivers/gpu/drm/i915/i915_reg.h | 4 +- drivers/gpu/drm/i915/intel_display.c | 2 + drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_pm.c | 22 - 5 files changed, 81 insertions(+), 104 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 49ad5fb82ace..8d8d33d068dd 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -997,129 +997,84 @@ static void notify_ring(struct drm_device *dev, wake_up_all(ring-irq_queue); } -static u32 vlv_c0_residency(struct drm_i915_private *dev_priv, - struct intel_rps_ei *rps_ei) +static void vlv_c0_read(struct drm_i915_private *dev_priv, + struct intel_rps_ei *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_clock == 0) { - rps_ei-cz_clock = cz_ts; - rps_ei-render_c0 = render_count; - rps_ei-media_c0 = media_count; - - return dev_priv-rps.cur_freq; - } - - elapsed_time = cz_ts - rps_ei-cz_clock; - rps_ei-cz_clock = cz_ts; + ei-cz_clock = vlv_punit_read(dev_priv, PUNIT_REG_CZ_TIMESTAMP); + ei-render_c0 = I915_READ(VLV_RENDER_C0_COUNT); + ei-media_c0 = I915_READ(VLV_MEDIA_C0_COUNT); +} - elapsed_render = render_count - rps_ei-render_c0; - rps_ei-render_c0 = render_count; +static bool vlv_c0_above(struct drm_i915_private *dev_priv, +const struct intel_rps_ei *old, +const struct intel_rps_ei *now, +int threshold) +{ + u64 time, c0; - elapsed_media = media_count - rps_ei-media_c0; - rps_ei-media_c0 = media_count; + if (old-cz_clock == 0) + return false; - /* 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; + time = now-cz_clock - old-cz_clock; + time *= threshold * dev_priv-mem_freq; - /* -* Calculate overall C0 residency percentage -* only if elapsed time is non zero + /* Workload can be split between render + media, e.g. SwapBuffers +* being blitted in X after being rendered in mesa. To account for +* this we need to combine both engines into our activity counter. */ - if (elapsed_time) { - residency = - ((max(elapsed_render, elapsed_media) * 100) - / elapsed_time); - } + c0 = now-render_c0 - old-render_c0; + c0 += now-media_c0 - old-media_c0; + c0 *= 100 * VLV_CZ_CLOCK_TO_MILLI_SEC * 4 / 1000; - return residency; + return c0 = time; } -/** - * 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 int vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv) +void gen6_rps_reset_ei(struct drm_i915_private *dev_priv) { - u32 residency_C0_up = 0, residency_C0_down = 0; - int new_delay, adj; - - dev_priv-rps.ei_interrupt_count++; - - WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock)); + vlv_c0_read(dev_priv, dev_priv-rps.down_ei); + dev_priv-rps.up_ei = dev_priv-rps.down_ei; + dev_priv-rps.ei_interrupt_count = 0; +} +static u32 vlv_wa_c0_ei(struct
[Intel-gfx] [PATCH v2 7/7] drm/i915: Boost GPU frequency if we detect outstanding pageflips
If we hit a vblank and see that have a pageflip queue but not yet processed, ensure that the GPU is running at maximum in order to clear the backlog. Pageflips are only queued for the following vblank, if we miss it, there will be a visible stutter. Boosting the GPU frequency doesn't prevent us from missing the target vblank, but it should help the subsequent frames hitting theirs. v2: Reorder vblank vs flip-complete so that we only check for a missed flip after processing the completion events, and avoid spurious boosts. v3: Rename missed_vblank v4: Rebase v5: Cancel the outstanding work in runtime suspend v6: Rebase v7: Rebase required fixing Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Deepak Sdeepa...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 11 --- drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_pm.c | 35 +++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f1c0295f69e5..0efb19a9b9a5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9852,6 +9852,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe) struct drm_i915_private *dev_priv = dev-dev_private; struct drm_crtc *crtc = dev_priv-pipe_to_crtc_mapping[pipe]; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_unpin_work *work; WARN_ON(!in_interrupt()); @@ -9859,12 +9860,16 @@ void intel_check_page_flip(struct drm_device *dev, int pipe) return; spin_lock(dev-event_lock); - if (intel_crtc-unpin_work __intel_pageflip_stall_check(dev, crtc)) { + work = intel_crtc-unpin_work; + if (work != NULL __intel_pageflip_stall_check(dev, crtc)) { WARN_ONCE(1, Kicking stuck page flip: queued at %d, now %d\n, -intel_crtc-unpin_work-flip_queued_vblank, -drm_vblank_count(dev, pipe)); +work-flip_queued_vblank, drm_vblank_count(dev, pipe)); page_flip_completed(intel_crtc); + work = NULL; } + if (work != NULL + drm_vblank_count(dev, pipe) - work-flip_queued_vblank 1) + intel_queue_rps_boost_for_request(dev, work-flip_queued_req); spin_unlock(dev-event_lock); } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8bb18e507f5f..d6e7ac8c2284 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1246,6 +1246,8 @@ void gen6_rps_busy(struct drm_i915_private *dev_priv); void gen6_rps_reset_ei(struct drm_i915_private *dev_priv); void gen6_rps_idle(struct drm_i915_private *dev_priv); void gen6_rps_boost(struct drm_i915_private *dev_priv); +void intel_queue_rps_boost_for_request(struct drm_device *dev, + struct drm_i915_gem_request *rq); void ilk_wm_get_hw_state(struct drm_device *dev); void skl_wm_get_hw_state(struct drm_device *dev); void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv, diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8b16bb3ae09f..e8111be32ed0 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6751,6 +6751,41 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val) return val / GT_FREQUENCY_MULTIPLIER; } +struct request_boost { + struct work_struct work; + struct drm_i915_gem_request *rq; +}; + +static void __intel_rps_boost_work(struct work_struct *work) +{ + struct request_boost *boost = container_of(work, struct request_boost, work); + + if (!i915_gem_request_completed(boost-rq, true)) + gen6_rps_boost(to_i915(boost-rq-ring-dev)); + + i915_gem_request_unreference__unlocked(boost-rq); + kfree(boost); +} + +void intel_queue_rps_boost_for_request(struct drm_device *dev, + struct drm_i915_gem_request *rq) +{ + struct request_boost *boost; + + if (rq == NULL || INTEL_INFO(dev)-gen 6) + return; + + boost = kmalloc(sizeof(*boost), GFP_ATOMIC); + if (boost == NULL) + return; + + i915_gem_request_reference(rq); + boost-rq = rq; + + INIT_WORK(boost-work, __intel_rps_boost_work); + queue_work(to_i915(dev)-wq, boost-work); +} + void intel_pm_setup(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 5/7] drm/i915: Fix computation of last_adjustment for RPS autotuning
The issue is that by computing the last_adj value after applying the clamping, we can end up with a bogus value for feeding into the next RPS autotuning step. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Deepak S deepa...@linux.intel.com Reviewed-by: Deepak S deepa...@linux.intel.com --- drivers/gpu/drm/i915/i915_irq.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 58af8e239971..8bcadfe8f00c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1096,21 +1096,20 @@ static void gen6_pm_rps_work(struct work_struct *work) pm_iir |= vlv_wa_c0_ei(dev_priv, pm_iir); adj = dev_priv-rps.last_adj; + new_delay = dev_priv-rps.cur_freq; if (pm_iir GEN6_PM_RP_UP_THRESHOLD) { if (adj 0) adj *= 2; - else { - /* CHV needs even encode values */ - adj = IS_CHERRYVIEW(dev_priv-dev) ? 2 : 1; - } - new_delay = dev_priv-rps.cur_freq + adj; - + else /* CHV needs even encode values */ + adj = IS_CHERRYVIEW(dev_priv) ? 2 : 1; /* * For better performance, jump directly * to RPe if we're below it. */ - if (new_delay dev_priv-rps.efficient_freq) + if (new_delay dev_priv-rps.efficient_freq - adj) { new_delay = dev_priv-rps.efficient_freq; + adj = 0; + } } else if (pm_iir GEN6_PM_RP_DOWN_TIMEOUT) { if (dev_priv-rps.cur_freq dev_priv-rps.efficient_freq) new_delay = dev_priv-rps.efficient_freq; @@ -1120,24 +1119,22 @@ static void gen6_pm_rps_work(struct work_struct *work) } else if (pm_iir GEN6_PM_RP_DOWN_THRESHOLD) { if (adj 0) adj *= 2; - else { - /* CHV needs even encode values */ - adj = IS_CHERRYVIEW(dev_priv-dev) ? -2 : -1; - } - new_delay = dev_priv-rps.cur_freq + adj; + else /* CHV needs even encode values */ + adj = IS_CHERRYVIEW(dev_priv) ? -2 : -1; } else { /* unknown event */ - new_delay = dev_priv-rps.cur_freq; + adj = 0; } + dev_priv-rps.last_adj = adj; + /* sysfs frequency interfaces may have snuck in while servicing the * interrupt */ + new_delay += adj; new_delay = clamp_t(int, new_delay, dev_priv-rps.min_freq_softlimit, dev_priv-rps.max_freq_softlimit); - dev_priv-rps.last_adj = new_delay - dev_priv-rps.cur_freq; - intel_set_rps(dev_priv-dev, new_delay); mutex_unlock(dev_priv-rps.hw_lock); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 4/7] drm/i915: Agressive downclocking on Baytrail
Reuse the same reclocking strategy for Baytail as on its bigger brethren, Sandybridge and Ivybridge. In particular, this makes the device quicker to reclock (both up and down) though the tendency now is to downclock more aggressively to compensate for the RPS boosts. v2: Rebase Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Deepak S deepa...@linux.intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Rodrigo Vivi rodrigo.v...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_irq.c | 4 ++-- drivers/gpu/drm/i915/i915_reg.h | 2 -- drivers/gpu/drm/i915/intel_pm.c | 7 ++- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b156bc30c9c9..afb552c1a4f8 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1031,6 +1031,9 @@ struct intel_gen6_power_mgmt { u8 rp0_freq;/* Non-overclocked max frequency. */ u32 cz_freq; + u8 up_threshold; /* Current %busy required to uplock */ + u8 down_threshold; /* Current %busy required to downclock */ + int last_adj; enum { LOW_POWER, BETWEEN, HIGH_POWER } power; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 6d8340d5a111..58af8e239971 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1050,7 +1050,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) if (pm_iir GEN6_PM_RP_DOWN_EI_EXPIRED) { if (!vlv_c0_above(dev_priv, dev_priv-rps.down_ei, now, - VLV_RP_DOWN_EI_THRESHOLD)) + dev_priv-rps.down_threshold)) events |= GEN6_PM_RP_DOWN_THRESHOLD; dev_priv-rps.down_ei = now; } @@ -1058,7 +1058,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) if (pm_iir GEN6_PM_RP_UP_EI_EXPIRED) { if (vlv_c0_above(dev_priv, dev_priv-rps.up_ei, now, -VLV_RP_UP_EI_THRESHOLD)) +dev_priv-rps.up_threshold)) events |= GEN6_PM_RP_UP_THRESHOLD; dev_priv-rps.up_ei = now; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5b84ee686f99..c94c06b21052 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -671,8 +671,6 @@ enum skl_disp_power_wells { #define FB_FMAX_VMIN_FREQ_LO_MASK0xf800 #define VLV_CZ_CLOCK_TO_MILLI_SEC 10 -#define VLV_RP_UP_EI_THRESHOLD 90 -#define VLV_RP_DOWN_EI_THRESHOLD 70 /* vlv2 north clock has */ #define CCK_FUSE_REG 0x8 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index e18f0fd22cf2..8b16bb3ae09f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3914,6 +3914,8 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) GEN6_RP_DOWN_IDLE_AVG); dev_priv-rps.power = new_power; + dev_priv-rps.up_threshold = threshold_up; + dev_priv-rps.down_threshold = threshold_down; dev_priv-rps.last_adj = 0; } @@ -3985,8 +3987,10 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val) Odd GPU freq value\n)) val = ~1; - if (val != dev_priv-rps.cur_freq) + if (val != dev_priv-rps.cur_freq) { vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val); + gen6_set_rps_thresholds(dev_priv, val); + } I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val)); @@ -4035,6 +4039,7 @@ static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) GENFREQSTATUS) == 0, 100)) DRM_ERROR(timed out waiting for Punit\n); + gen6_set_rps_thresholds(dev_priv, val); vlv_force_gfx_clock(dev_priv, false); I915_WRITE(GEN6_PMINTRMSK, gen6_rps_pm_mask(dev_priv, val)); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] RPS tuning for VLV and pageflips
A few r-bs and suggestions from Deepak rebased onto -nightly. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 6/7] drm/i915: Add i915_gem_request_unreference__unlocked
We were missing a convenience stub to aquire the right mutex whilst dropping the request, so add it. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_drv.h | 13 + drivers/gpu/drm/i915/i915_gem.c | 8 ++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index afb552c1a4f8..eaf21605738f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2143,6 +2143,19 @@ i915_gem_request_unreference(struct drm_i915_gem_request *req) kref_put(req-ref, i915_gem_request_free); } +static inline void +i915_gem_request_unreference__unlocked(struct drm_i915_gem_request *req) +{ + if (req !atomic_add_unless(req-ref.refcount, -1, 1)) { + struct drm_device *dev = req-ring-dev; + + mutex_lock(dev-struct_mutex); + if (likely(atomic_dec_and_test(req-ref.refcount))) + i915_gem_request_free(req-ref); + mutex_unlock(dev-struct_mutex); + } +} + static inline void i915_gem_request_assign(struct drm_i915_gem_request **pdst, struct drm_i915_gem_request *src) { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index e2876bf83da7..3df65fc2aa74 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2957,9 +2957,7 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file) ret = __i915_wait_request(req, reset_counter, true, args-timeout_ns 0 ? args-timeout_ns : NULL, file-driver_priv); - mutex_lock(dev-struct_mutex); - i915_gem_request_unreference(req); - mutex_unlock(dev-struct_mutex); + i915_gem_request_unreference__unlocked(req); return ret; out: @@ -4146,9 +4144,7 @@ i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file) if (ret == 0) queue_delayed_work(dev_priv-wq, dev_priv-mm.retire_work, 0); - mutex_lock(dev-struct_mutex); - i915_gem_request_unreference(target); - mutex_unlock(dev-struct_mutex); + i915_gem_request_unreference__unlocked(target); return ret; } -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Do not use ggtt_view with (aliasing) PPGTT
On Tue, Mar 17, 2015 at 02:19:18PM +, Tvrtko Ursulin wrote: Hi, On 03/16/2015 12:11 PM, Joonas Lahtinen wrote: GGTT views are only applicable when dealing with GGTT. Change the code to reject ggtt_view where it should not be used and require it when it should be. v2: - Dropped _ppgtt_ infixes, allow both types to be passed - Disregard other but normal views when no view is specified - More checks that valid parameters are passed - More readable error checking v3: - Prefer WARN_ONCE over BUG_ON when there is code path for failure [snip] +unsigned long +i915_gem_obj_ggtt_offset_view(struct drm_i915_gem_object *o, + enum i915_ggtt_view_type view) { +struct drm_i915_private *dev_priv = o-base.dev-dev_private; Unused variable slipped through, oops! :) Indeed, fixed up. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Agressive downclocking on Baytrail
On Wed, Mar 18, 2015 at 10:48:36AM +0100, Daniel Vetter wrote: On Wed, Mar 18, 2015 at 01:42:58PM +0530, Deepak S wrote: I guess your empty reply wasn't intentional? I heard This needs to be rebased against -nightly. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Missing notifications for front buffer status change(invalidate)
On Wed, Mar 18, 2015 at 02:23:01PM +0530, Ramalingam C wrote: cc += intel-gfx On Wednesday 18 March 2015 02:12 PM, Ramalingam C wrote: Hi, Is there anyway I can test the front buffer tracking? On latest drm-intel-nightly, I am observing that when ubuntu bootsup after few secs (like ~15Secs) there no call from intel_fb_obj_invalidate() to invalidate the drrs. But system is healthy and FBs are keep changing. But no notification to DRRS for FB status change. I have verified this by observing the busy_frontbuffer_bits from /sys/kernel/debug/dri/0/i915_drrs_status. This bit is not changing at all. Hence DRRS is stuck at DRRS_LOW_RR itself. Is it known valid issue? Or am I doing something wrong.!? Yeah that shouldn't happen indeed. Unfortunately we've created the frontbuffer code for fbc/psr, and that code is still not yet enabled by default. So testing for frontbuffer tracking is probably too thin. And we still discover corner-cases in the frontbuffer tracking logic. So just tracking down what's going and fix it. Plus perhaps we do indeed need a DRRS testcase to exercise these corner-cases and check that the kernel does correctly switch between low and high modes. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] tests/gem_exec_lut_handle
Reduce default number of repeats a lot. High repeat count is only useful for microbenchmarking, not that much for regression testing. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87131 Signed-off-by: Daniel Vetter daniel.vet...@intel.com --- tests/gem_exec_lut_handle.c | 38 +++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/tests/gem_exec_lut_handle.c b/tests/gem_exec_lut_handle.c index c2d490f94cd6..e2e31f233cd5 100644 --- a/tests/gem_exec_lut_handle.c +++ b/tests/gem_exec_lut_handle.c @@ -113,9 +113,32 @@ static int exec(int fd, int num_exec, int num_relocs, unsigned flags) execbuf); } +int repeats = 10; + +static int opt_handler(int opt, int opt_index) +{ + switch (opt) { + case 'r': + igt_warn(meh\n); + repeats = atoi(optarg); + igt_warn(meh\n); + break; + default: + igt_assert(0); + } + + return 0; +} + #define ELAPSED(a,b) (1e6*((b)-tv_sec - (a)-tv_sec) + ((b)-tv_usec - (a)-tv_usec)) -igt_simple_main +int main(int argc, char **argv) { + const char *help_str = +--repeats\t\tNumber of repeats to run the microbenchmarks for.; + static struct option long_options[] = { + {repeats, 1, 0, 'r'}, + { 0, 0, 0, 0 } + }; uint32_t batch[2] = {MI_BATCH_BUFFER_END}; int fd, n, m, count; const struct { @@ -128,6 +151,9 @@ igt_simple_main { .name = NULL }, }, *p; + igt_simple_init_parse_opts(argc, argv, , long_options, + help_str, opt_handler); + igt_skip_on_simulation(); fd = drm_open_any(); @@ -160,7 +186,7 @@ igt_simple_main do_or_die(exec(fd, n, m, 0 | p-flags)); gettimeofday(start, NULL); - for (count = 0; count 1000; count++) + for (count = 0; count repeats; count++) do_or_die(exec(fd, n, m, 0 | p-flags)); gettimeofday(end, NULL); gem_sync(fd, gem_exec[MAX_NUM_EXEC].handle); @@ -168,7 +194,7 @@ igt_simple_main do_or_die(exec(fd, n, m, USE_LUT | p-flags)); gettimeofday(start, NULL); - for (count = 0; count 1000; count++) + for (count = 0; count repeats; count++) do_or_die(exec(fd, n, m, USE_LUT | p-flags)); gettimeofday(end, NULL); gem_sync(fd, gem_exec[MAX_NUM_EXEC].handle); @@ -204,4 +230,10 @@ igt_simple_main igt_info(\n); } } + + igt_info(Used %i rounds for testing.\n +Increase for more accurate results when microbenchmarking with --repeats\n, +repeats); + + igt_exit(); } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 19/21] drm/i915: Enable skylake panel fitting using skylake shared scalers
On Tue, Mar 17, 2015 at 08:43:02PM +, Konduru, Chandra wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, March 17, 2015 7:22 AM To: Konduru, Chandra Cc: intel-gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander; Vetter, Daniel Subject: Re: [Intel-gfx] [PATCH 19/21] drm/i915: Enable skylake panel fitting using skylake shared scalers On Sat, Mar 14, 2015 at 10:55:44PM -0700, Chandra Konduru wrote: Modify skylake panel fitting implementation to use shared scalers. Signed-off-by: Chandra Konduru chandra.kond...@intel.com Ah here's the real pfit state, and the scaler state is just readout. Still we recover this from the bios, I do think we need to make sure that at least the crtc scaler is correctly assigned. Can't we recompute the scaler state after readout to make sure it fits with pfit state here? -Daniel Crtc scaler can be computed from hw readout. As replied earlier, will be making changes to update crtc scaler_id and appropriately set the scaler_users. When you add state readout suppoort for crtc scaler id, please also add corresponding self-check code to intel_pipe_config_compare. That way we can exercise all the fastboot code with normal igt testcases which massively improves automated test coverage. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [patch] drm/i915: memory leak in __i915_gem_vma_create()
On Wed, 18 Mar 2015, Dan Carpenter dan.carpen...@oracle.com wrote: In the original code then if WARN_ON(i915_is_ggtt(vm) != !!ggtt_view) was true then we leak vma. Presumably that doesn't happen often but static checkers complain and this bug is easy to fix. Fixes: c3bbb6f2825d ('drm/i915: Do not use ggtt_view with (aliasing) PPGTT') Signed-off-by: Dan Carpenter dan.carpen...@oracle.com Reviewed-by: Jani Nikula jani.nik...@intel.com diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index f1b9ea6..cbf013f 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2340,12 +2340,13 @@ __i915_gem_vma_create(struct drm_i915_gem_object *obj, struct i915_address_space *vm, const struct i915_ggtt_view *ggtt_view) { - struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL); - if (vma == NULL) - return ERR_PTR(-ENOMEM); + struct i915_vma *vma; if (WARN_ON(i915_is_ggtt(vm) != !!ggtt_view)) return ERR_PTR(-EINVAL); + vma = kzalloc(sizeof(*vma), GFP_KERNEL); + if (vma == NULL) + return ERR_PTR(-ENOMEM); INIT_LIST_HEAD(vma-vma_link); INIT_LIST_HEAD(vma-mm_list); -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/7] drm/i915: Relax RPS contraints to allows setting minfreq on idle
On Wed, Mar 18, 2015 at 11:14:15AM +0530, Deepak S wrote: @@ -4761,6 +4762,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev) I think we missed initializing idle_freq in valleyview init platform? Indeed, I did. Thanks, -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/12] drm/i915/bdw: Make pdp allocation more dynamic
On 3/3/2015 11:48 AM, akash goel wrote: On Fri, Feb 20, 2015 at 11:15 PM, Michel Thierry michel.thie...@intel.com wrote: From: Ben Widawskybenjamin.widaw...@intel.com This transitional patch doesn't do much for the existing code. However, it should make upcoming patches to use the full 48b address space a bit easier to swallow. The patch also introduces the PML4, ie. the new top level structure of the page tables. v2: Renamed pdp_free to be similar to pd/pt (unmap_and_free_pdp), To facilitate testing, 48b mode will be available on Broadwell, when i915.enable_ppgtt = 3. Signed-off-by: Ben Widawskyb...@bwidawsk.net Signed-off-by: Michel Thierrymichel.thie...@intel.com (v2) --- drivers/gpu/drm/i915/i915_drv.h | 7 ++- drivers/gpu/drm/i915/i915_gem_gtt.c | 108 +--- drivers/gpu/drm/i915/i915_gem_gtt.h | 41 +++--- 3 files changed, 126 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2dedd43..af0d149 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2432,7 +2432,12 @@ struct drm_i915_cmd_table { #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)-gen = 6) #define HAS_LOGICAL_RING_CONTEXTS(dev) (INTEL_INFO(dev)-gen = 8) #define USES_PPGTT(dev)(i915.enable_ppgtt) -#define USES_FULL_PPGTT(dev) (i915.enable_ppgtt == 2) +#define USES_FULL_PPGTT(dev) (i915.enable_ppgtt = 2) +#ifdef CONFIG_64BIT +# define USES_FULL_48BIT_PPGTT(dev)(i915.enable_ppgtt == 3) +#else +# define USES_FULL_48BIT_PPGTT(dev)false +#endif #define HAS_OVERLAY(dev) (INTEL_INFO(dev)-has_overlay) #define OVERLAY_NEEDS_PHYSICAL(dev) (INTEL_INFO(dev)-overlay_needs_physical) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index ff86501..489f8db 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -100,10 +100,17 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt) { bool has_aliasing_ppgtt; bool has_full_ppgtt; + bool has_full_64bit_ppgtt; has_aliasing_ppgtt = INTEL_INFO(dev)-gen = 6; has_full_ppgtt = INTEL_INFO(dev)-gen = 7; +#ifdef CONFIG_64BIT + has_full_64bit_ppgtt = IS_BROADWELL(dev) false; /* FIXME: 64b */ +#else + has_full_64bit_ppgtt = false; +#endif + if (intel_vgpu_active(dev)) has_full_ppgtt = false; /* emulation is too hard */ @@ -121,6 +128,9 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt) if (enable_ppgtt == 2 has_full_ppgtt) return 2; + if (enable_ppgtt == 3 has_full_64bit_ppgtt) + return 3; + #ifdef CONFIG_INTEL_IOMMU /* Disable ppgtt on SNB if VT-d is on. */ if (INTEL_INFO(dev)-gen == 6 intel_iommu_gfx_mapped) { @@ -462,6 +472,45 @@ free_pd: return ERR_PTR(ret); } +static void __pdp_fini(struct i915_page_directory_pointer_entry *pdp) +{ + kfree(pdp-used_pdpes); + kfree(pdp-page_directory); + /* HACK */ + pdp-page_directory = NULL; +} + +static void unmap_and_free_pdp(struct i915_page_directory_pointer_entry *pdp, + struct drm_device *dev) +{ + __pdp_fini(pdp); + if (USES_FULL_48BIT_PPGTT(dev)) + kfree(pdp); +} + +static int __pdp_init(struct i915_page_directory_pointer_entry *pdp, + struct drm_device *dev) +{ + size_t pdpes = I915_PDPES_PER_PDP(dev); + + pdp-used_pdpes = kcalloc(BITS_TO_LONGS(pdpes), + sizeof(unsigned long), + GFP_KERNEL); + if (!pdp-used_pdpes) + return -ENOMEM; + + pdp-page_directory = kcalloc(pdpes, sizeof(*pdp-page_directory), GFP_KERNEL); + if (!pdp-page_directory) { + kfree(pdp-used_pdpes); + /* the PDP might be the statically allocated top level. Keep it +* as clean as possible */ + pdp-used_pdpes = NULL; + return -ENOMEM; + } + + return 0; +} + /* Broadwell Page Directory Pointer Descriptors */ static int gen8_write_pdp(struct intel_engine_cs *ring, unsigned entry, @@ -491,7 +540,7 @@ static int gen8_mm_switch(struct i915_hw_ppgtt *ppgtt, { int i, ret; - for (i = GEN8_LEGACY_PDPES - 1; i = 0; i--) { + for (i = 3; i = 0; i--) { struct i915_page_directory_entry *pd = ppgtt-pdp.page_directory[i]; dma_addr_t pd_daddr = pd ? pd-daddr : ppgtt-scratch_pd-daddr; /* The page directory might be NULL, but we need to clear out @@ -580,9 +629,6 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm, pt_vaddr = NULL; for_each_sg_page(pages-sgl, sg_iter, pages-nents, 0) { -
Re: [Intel-gfx] [PATCH 02/12] drm/i915/bdw: Abstract PDP usage
On 3/3/2015 12:16 PM, akash goel wrote: On Fri, Feb 20, 2015 at 11:15 PM, Michel Thierry michel.thie...@intel.com wrote: From: Ben Widawskybenjamin.widaw...@intel.com Up until now, ppgtt-pdp has always been the root of our page tables. Legacy 32b addresses acted like it had 1 PDP with 4 PDPEs. In preparation for 4 level page tables, we need to stop use ppgtt-pdp directly unless we know it's what we want. The future structure will use ppgtt-pml4 for the top level, and the pdp is just one of the entries being pointed to by a pml4e. v2: Updated after dynamic page allocation changes. Signed-off-by: Ben Widawskyb...@bwidawsk.net Signed-off-by: Michel Thierrymichel.thie...@intel.com (v2) --- drivers/gpu/drm/i915/i915_gem_gtt.c | 123 1 file changed, 70 insertions(+), 53 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 489f8db..d3ad517 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -560,6 +560,7 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm, { struct i915_hw_ppgtt *ppgtt = container_of(vm, struct i915_hw_ppgtt, base); + struct i915_page_directory_pointer_entry *pdp = ppgtt-pdp; /* FIXME: 48b */ gen8_gtt_pte_t *pt_vaddr, scratch_pte; unsigned pdpe = start GEN8_PDPE_SHIFT GEN8_PDPE_MASK; unsigned pde = start GEN8_PDE_SHIFT GEN8_PDE_MASK; @@ -575,10 +576,10 @@ static void gen8_ppgtt_clear_range(struct i915_address_space *vm, struct i915_page_table_entry *pt; struct page *page_table; - if (WARN_ON(!ppgtt-pdp.page_directory[pdpe])) + if (WARN_ON(!pdp-page_directory[pdpe])) continue; - pd = ppgtt-pdp.page_directory[pdpe]; + pd = pdp-page_directory[pdpe]; if (WARN_ON(!pd-page_tables[pde])) continue; @@ -620,6 +621,7 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm, { struct i915_hw_ppgtt *ppgtt = container_of(vm, struct i915_hw_ppgtt, base); + struct i915_page_directory_pointer_entry *pdp = ppgtt-pdp; /* FIXME: 48b */ gen8_gtt_pte_t *pt_vaddr; unsigned pdpe = start GEN8_PDPE_SHIFT GEN8_PDPE_MASK; unsigned pde = start GEN8_PDE_SHIFT GEN8_PDE_MASK; @@ -630,7 +632,7 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm, for_each_sg_page(pages-sgl, sg_iter, pages-nents, 0) { if (pt_vaddr == NULL) { - struct i915_page_directory_entry *pd = ppgtt-pdp.page_directory[pdpe]; + struct i915_page_directory_entry *pd = pdp-page_directory[pdpe]; struct i915_page_table_entry *pt = pd-page_tables[pde]; struct page *page_table = pt-page; @@ -708,16 +710,17 @@ static void gen8_free_page_tables(struct i915_page_directory_entry *pd, struct d static void gen8_ppgtt_unmap_pages(struct i915_hw_ppgtt *ppgtt) { struct pci_dev *hwdev = ppgtt-base.dev-pdev; + struct i915_page_directory_pointer_entry *pdp = ppgtt-pdp; /* FIXME: 48b */ int i, j; - for_each_set_bit(i, ppgtt-pdp.used_pdpes, + for_each_set_bit(i, pdp-used_pdpes, I915_PDPES_PER_PDP(ppgtt-base.dev)) { struct i915_page_directory_entry *pd; - if (WARN_ON(!ppgtt-pdp.page_directory[i])) + if (WARN_ON(!pdp-page_directory[i])) continue; - pd = ppgtt-pdp.page_directory[i]; + pd = pdp-page_directory[i]; if (!pd-daddr) pci_unmap_page(hwdev, pd-daddr, PAGE_SIZE, PCI_DMA_BIDIRECTIONAL); @@ -743,15 +746,21 @@ static void gen8_ppgtt_free(struct i915_hw_ppgtt *ppgtt) { int i; - for_each_set_bit(i, ppgtt-pdp.used_pdpes, - I915_PDPES_PER_PDP(ppgtt-base.dev)) { - if (WARN_ON(!ppgtt-pdp.page_directory[i])) - continue; + if (!USES_FULL_48BIT_PPGTT(ppgtt-base.dev)) { + for_each_set_bit(i, ppgtt-pdp.used_pdpes, +I915_PDPES_PER_PDP(ppgtt-base.dev)) { + if (WARN_ON(!ppgtt-pdp.page_directory[i])) + continue; - gen8_free_page_tables(ppgtt-pdp.page_directory[i], ppgtt-base.dev); - unmap_and_free_pd(ppgtt-pdp.page_directory[i], ppgtt-base.dev); + gen8_free_page_tables(ppgtt-pdp.page_directory[i], + ppgtt-base.dev); + unmap_and_free_pd(ppgtt-pdp.page_directory[i], + ppgtt-base.dev); + } +
Re: [Intel-gfx] [PATCH v2 4/7] drm/i915: Agressive downclocking on Baytrail
On Wed, Mar 18, 2015 at 04:45:08PM +0530, Deepak S wrote: +if (val != dev_priv-rps.cur_freq) { vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val); +gen6_set_rps_thresholds(dev_priv, val); I think gen6_set_rps_thresholds should be under baytrail specific with platform check? The only difference for cherryview is that it doesn't use GEN6_RP_MEDIA_TURBO. Was that intentional? The whole idea is that RPS should be autotuning for different workloads, but those metrics are equivalent across GPU. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/7] drm/i915: Agressive downclocking on Baytrail
On Friday 06 March 2015 08:36 PM, Chris Wilson wrote: Reuse the same reclocking strategy for Baytail as on its bigger brethren, Sandybridge and Ivybridge. In particular, this makes the device quicker to reclock (both up and down) though the tendency now is to downclock more aggressively to compensate for the RPS boosts. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Deepak S deepa...@linux.intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Rodrigo Vivi rodrigo.v...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Conflicts: drivers/gpu/drm/i915/intel_pm.c --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_irq.c | 4 ++-- drivers/gpu/drm/i915/i915_reg.h | 2 -- drivers/gpu/drm/i915/intel_pm.c | 11 ++- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index efa98c9e5777..bfa6e11f802e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1028,6 +1028,9 @@ struct intel_gen6_power_mgmt { u8 rp0_freq;/* Non-overclocked max frequency. */ u32 cz_freq; + u8 up_threshold; /* Current %busy required to uplock */ + u8 down_threshold; /* Current %busy required to downclock */ + int last_adj; enum { LOW_POWER, BETWEEN, HIGH_POWER } power; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 8892dbdfb629..483079d96957 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1050,7 +1050,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) if (pm_iir GEN6_PM_RP_DOWN_EI_EXPIRED) { if (!vlv_c0_above(dev_priv, dev_priv-rps.down_ei, now, - VLV_RP_DOWN_EI_THRESHOLD)) + dev_priv-rps.down_threshold)) events |= GEN6_PM_RP_DOWN_THRESHOLD; dev_priv-rps.down_ei = now; } @@ -1058,7 +1058,7 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) if (pm_iir GEN6_PM_RP_UP_EI_EXPIRED) { if (vlv_c0_above(dev_priv, dev_priv-rps.up_ei, now, -VLV_RP_UP_EI_THRESHOLD)) +dev_priv-rps.up_threshold)) events |= GEN6_PM_RP_UP_THRESHOLD; dev_priv-rps.up_ei = now; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ca24a2d4a823..13ec6c2b1fcf 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -663,8 +663,6 @@ enum skl_disp_power_wells { #define FB_FMAX_VMIN_FREQ_LO_MASK 0xf800 #define VLV_CZ_CLOCK_TO_MILLI_SEC 10 -#define VLV_RP_UP_EI_THRESHOLD 90 -#define VLV_RP_DOWN_EI_THRESHOLD 70 /* vlv2 north clock has */ #define CCK_FUSE_REG 0x8 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 55dc406cd195..972333d2211d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3722,10 +3722,12 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) switch (new_power) { case LOW_POWER: /* Upclock if more than 95% busy over 16ms */ + dev_priv-rps.up_threshold = 95; I915_WRITE(GEN6_RP_UP_EI, 12500); I915_WRITE(GEN6_RP_UP_THRESHOLD, 11800); /* Downclock if less than 85% busy over 32ms */ + dev_priv-rps.down_threshold = 85; I915_WRITE(GEN6_RP_DOWN_EI, 25000); I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 21250); @@ -3740,10 +3742,12 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) case BETWEEN: /* Upclock if more than 90% busy over 13ms */ + dev_priv-rps.up_threshold = 90; I915_WRITE(GEN6_RP_UP_EI, 10250); I915_WRITE(GEN6_RP_UP_THRESHOLD, 9225); /* Downclock if less than 75% busy over 32ms */ + dev_priv-rps.down_threshold = 75; I915_WRITE(GEN6_RP_DOWN_EI, 25000); I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 18750); @@ -3758,10 +3762,12 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) case HIGH_POWER: /* Upclock if more than 85% busy over 10ms */ + dev_priv-rps.up_threshold = 85; I915_WRITE(GEN6_RP_UP_EI, 8000); I915_WRITE(GEN6_RP_UP_THRESHOLD, 6800); /* Downclock if less than 60% busy over 32ms */ + dev_priv-rps.down_threshold = 60; I915_WRITE(GEN6_RP_DOWN_EI, 25000); I915_WRITE(GEN6_RP_DOWN_THRESHOLD, 15000); @@
Re: [Intel-gfx] Missing notifications for front buffer status change(invalidate)
cc += intel-gfx On Wednesday 18 March 2015 02:12 PM, Ramalingam C wrote: Hi, Is there anyway I can test the front buffer tracking? On latest drm-intel-nightly, I am observing that when ubuntu bootsup after few secs (like ~15Secs) there no call from intel_fb_obj_invalidate() to invalidate the drrs. But system is healthy and FBs are keep changing. But no notification to DRRS for FB status change. I have verified this by observing the busy_frontbuffer_bits from /sys/kernel/debug/dri/0/i915_drrs_status. This bit is not changing at all. Hence DRRS is stuck at DRRS_LOW_RR itself. Is it known valid issue? Or am I doing something wrong.!? -- Ram ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Extract i915_gem_shrinker.c
On Wed, Mar 18, 2015 at 10:46:04AM +0100, Daniel Vetter wrote: Two code changes: - Extract i915_gem_shrinker_init. - Inline i915_gem_object_is_purgeable since we open-code it everywhere else too. This already has the benefit of pulling all the shrinker code together, next patch adds a bit of kerneldoc. Signed-off-by: Daniel Vetter daniel.vet...@intel.com Worth it just for i915_gem_shrinker_init(). Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: kerneldoc for i915_gem_shrinker.c
On Wed, Mar 18, 2015 at 10:46:05AM +0100, Daniel Vetter wrote: +/** + * i915_gem_shrink - Shrink buffer object caches + * @dev_priv: i915 device + * @target: memory space to make available, in pages Not memory space, too similar to VM address space. @target: Amount of memory to make available, in pages + * @flags: control flags for selecting cache types + * + * This function is the main interface to the shrinker. It will try to release + * up to @target pages of memory backing storage from buffer objects. Mention unpinned? Certainly we should try to explain part of the interaction between pin_pages and pin here. (Though nowadays we have clarified that slightly by pin explicitly doing a pin_pages.) We should also mention that not everything we release here will generate freed pages. (Hmm, though maybe we could change how we count to ensure target free pages are generated? It still wouldn't help get_pages_gtt as there is no guarantee that the pages we free will be given back to us...) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/7] drm/i915: Relax RPS contraints to allows setting minfreq on idle
On Wednesday 18 March 2015 03:18 PM, Chris Wilson wrote: When we idle, we set the GPU frequency to the hardware minimum (not user minimum). We introduce a new variable to distinguish between the different roles, and to allow easy tuning of the idle frequency without impacting over aspects of RPS. Setting the minimum frequency should be a safety blanket as the pcu on the GPU should be power gating itself anyway. However, in order for us to do set the absolute minimum frequency, we need to relax a few of our assertions that we do not exceed the user limits. v2: Add idle_freq v3: Init idle_freq for vlv and add a bunch of WARNs Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Deepak S deepa...@linux.intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 6 + drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_pm.c | 44 +++-- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index aaf756047a20..007c7d7d8295 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1200,6 +1200,9 @@ static int i915_frequency_info(struct seq_file *m, void *unused) seq_printf(m, Max overclocked frequency: %dMHz\n, intel_gpu_freq(dev_priv, dev_priv-rps.max_freq)); + + seq_printf(m, Idle freq: %d MHz\n, + intel_gpu_freq(dev_priv, dev_priv-rps.idle_freq)); } else if (IS_VALLEYVIEW(dev)) { u32 freq_sts; @@ -1214,6 +1217,9 @@ static int i915_frequency_info(struct seq_file *m, void *unused) seq_printf(m, min GPU freq: %d MHz\n, intel_gpu_freq(dev_priv, dev_priv-rps.min_freq)); + seq_printf(m, idle GPU freq: %d MHz\n, + intel_gpu_freq(dev_priv, dev_priv-rps.idle_freq)); + seq_printf(m, efficient (RPe) frequency: %d MHz\n, intel_gpu_freq(dev_priv, dev_priv-rps.efficient_freq)); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 81f60b48def2..a06536cfce6d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1025,6 +1025,7 @@ struct intel_gen6_power_mgmt { u8 max_freq_softlimit; /* Max frequency permitted by the driver */ u8 max_freq;/* Maximum frequency, RP0 if not overclocking */ u8 min_freq;/* AKA RPn. Minimum frequency */ + u8 idle_freq; /* Frequency to request when we are idle */ u8 efficient_freq; /* AKA RPe. Pre-determined balanced frequency */ u8 rp1_freq;/* less than RP0 power/freqency */ u8 rp0_freq;/* Non-overclocked max frequency. */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 288c9d24098e..beab305e320d 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3855,9 +3855,9 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) break; } /* Max/min bins are special */ - if (val == dev_priv-rps.min_freq_softlimit) + if (val = dev_priv-rps.min_freq_softlimit) new_power = LOW_POWER; - if (val == dev_priv-rps.max_freq_softlimit) + if (val = dev_priv-rps.max_freq_softlimit) new_power = HIGH_POWER; if (new_power == dev_priv-rps.power) return; @@ -3940,8 +3940,8 @@ static void gen6_set_rps(struct drm_device *dev, u8 val) struct drm_i915_private *dev_priv = dev-dev_private; WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock)); - WARN_ON(val dev_priv-rps.max_freq_softlimit); - WARN_ON(val dev_priv-rps.min_freq_softlimit); + WARN_ON(val dev_priv-rps.max_freq); + WARN_ON(val dev_priv-rps.min_freq); /* min/max delay may still have been modified so be sure to * write the limits value. @@ -3979,8 +3979,8 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val) struct drm_i915_private *dev_priv = dev-dev_private; WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock)); - WARN_ON(val dev_priv-rps.max_freq_softlimit); - WARN_ON(val dev_priv-rps.min_freq_softlimit); + WARN_ON(val dev_priv-rps.max_freq); + WARN_ON(val dev_priv-rps.min_freq); if (WARN_ONCE(IS_CHERRYVIEW(dev) (val 1), Odd GPU freq value\n)) @@ -4007,10 +4007,11 @@ static void valleyview_set_rps(struct drm_device *dev, u8 val) static void vlv_set_rps_idle(struct drm_i915_private *dev_priv) { struct drm_device *dev = dev_priv-dev; + u32 val = dev_priv-rps.idle_freq; /* CHV and latest VLV don't need to force the gfx clock */ if (IS_CHERRYVIEW(dev) || dev-pdev-revision = 0xd) { -
[Intel-gfx] [PATCH v4] drm/i915: Allocate a drm_atomic_state for the legacy modeset code
For the atomic conversion, the mode set paths need to be changed to rely on an atomic state instead of using the staged config. By using an atomic state for the legacy code, we will be able to convert the code base in small chunks. v2: Squash patch that adds stat argument to intel_set_mode(). (Ander) Make every caller of intel_set_mode() allocate state. (Daniel) Call drm_atomic_state_clear() in set config's error path. (Daniel) v3: Copy staged config to atomic state in force restore path. (Ander) v4: Don't update -new_config for disabled pipes in __intel_set_mode(), since it is expected to be NULL in that case. (Ander) Signed-off-by: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 200 +-- 1 file changed, 165 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8458bf5..ce35647 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -83,7 +83,8 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc, struct intel_crtc_state *pipe_config); static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode, - int x, int y, struct drm_framebuffer *old_fb); + int x, int y, struct drm_framebuffer *old_fb, + struct drm_atomic_state *state); static int intel_framebuffer_init(struct drm_device *dev, struct intel_framebuffer *ifb, struct drm_mode_fb_cmd2 *mode_cmd, @@ -8802,6 +8803,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector, struct drm_device *dev = encoder-dev; struct drm_framebuffer *fb; struct drm_mode_config *config = dev-mode_config; + struct drm_atomic_state *state = NULL; int ret, i = -1; DRM_DEBUG_KMS([CONNECTOR:%d:%s], [ENCODER:%d:%s]\n, @@ -8883,6 +8885,12 @@ retry: old-load_detect_temp = true; old-release_fb = NULL; + state = drm_atomic_state_alloc(dev); + if (!state) + return false; + + state-acquire_ctx = ctx; + if (!mode) mode = load_detect_mode; @@ -8905,7 +8913,7 @@ retry: goto fail; } - if (intel_set_mode(crtc, mode, 0, 0, fb)) { + if (intel_set_mode(crtc, mode, 0, 0, fb, state)) { DRM_DEBUG_KMS(failed to set mode on load-detect pipe\n); if (old-release_fb) old-release_fb-funcs-destroy(old-release_fb); @@ -8924,6 +8932,11 @@ retry: else intel_crtc-new_config = NULL; fail_unlock: + if (state) { + drm_atomic_state_free(state); + state = NULL; + } + if (ret == -EDEADLK) { drm_modeset_backoff(ctx); goto retry; @@ -8936,22 +8949,34 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, struct intel_load_detect_pipe *old, struct drm_modeset_acquire_ctx *ctx) { + struct drm_device *dev = connector-dev; struct intel_encoder *intel_encoder = intel_attached_encoder(connector); struct drm_encoder *encoder = intel_encoder-base; struct drm_crtc *crtc = encoder-crtc; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct drm_atomic_state *state; DRM_DEBUG_KMS([CONNECTOR:%d:%s], [ENCODER:%d:%s]\n, connector-base.id, connector-name, encoder-base.id, encoder-name); if (old-load_detect_temp) { + state = drm_atomic_state_alloc(dev); + if (!state) { + DRM_DEBUG_KMS(can't release load detect pipe\n); + return; + } + + state-acquire_ctx = ctx; + to_intel_connector(connector)-new_encoder = NULL; intel_encoder-new_crtc = NULL; intel_crtc-new_enabled = false; intel_crtc-new_config = NULL; - intel_set_mode(crtc, NULL, 0, 0, NULL); + intel_set_mode(crtc, NULL, 0, 0, NULL, state); + + drm_atomic_state_free(state); if (old-release_fb) { drm_framebuffer_unregister_private(old-release_fb); @@ -10345,10 +10370,22 @@ static bool check_digital_port_conflicts(struct drm_device *dev) return true; } -static struct intel_crtc_state * +static void +clear_intel_crtc_state(struct intel_crtc_state *crtc_state) +{ + struct drm_crtc_state tmp_state; + + /* Clear only the intel specific part of the crtc state */ + tmp_state = crtc_state-base; + memset(crtc_state, 0, sizeof *crtc_state);
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Improved w/a for rps on Baytrail
On Friday 06 March 2015 08:36 PM, Chris Wilson wrote: Rewrite commit 31685c258e0b0ad6aa486c5ec001382cf8a64212 Author: Deepak S deepa...@linux.intel.com Date: Thu Jul 3 17:33:01 2014 -0400 drm/i915/vlv: WA for Turbo and RC6 to work together. Other than code clarity, the major improvement is to disable the extra interrupts generated when idle. However, the reclocking remains rather slow under the new manual regime, in particular it fails to downclock as quickly as desired. The second major improvement is that for certain workloads, like games, we need to combine render+media activity counters as the work of displaying the frame is split across the engines and both need to be taken into account when deciding the global GPU frequency as memory cycles are shared. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Deepak S deepa...@linux.intel.com Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Rodrigo Vivi rodrigo.v...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Conflicts: drivers/gpu/drm/i915/intel_pm.c --- drivers/gpu/drm/i915/i915_irq.c | 155 +-- drivers/gpu/drm/i915/i915_reg.h | 4 +- drivers/gpu/drm/i915/intel_display.c | 2 + drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_pm.c | 22 - 5 files changed, 81 insertions(+), 104 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 1296ce37e435..4b7b86298b37 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -997,129 +997,84 @@ static void notify_ring(struct drm_device *dev, wake_up_all(ring-irq_queue); } -static u32 vlv_c0_residency(struct drm_i915_private *dev_priv, - struct intel_rps_ei *rps_ei) +static void vlv_c0_read(struct drm_i915_private *dev_priv, + struct intel_rps_ei *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_clock == 0) { - rps_ei-cz_clock = cz_ts; - rps_ei-render_c0 = render_count; - rps_ei-media_c0 = media_count; - - return dev_priv-rps.cur_freq; - } - - elapsed_time = cz_ts - rps_ei-cz_clock; - rps_ei-cz_clock = cz_ts; + ei-cz_clock = vlv_punit_read(dev_priv, PUNIT_REG_CZ_TIMESTAMP); + ei-render_c0 = I915_READ(VLV_RENDER_C0_COUNT); + ei-media_c0 = I915_READ(VLV_MEDIA_C0_COUNT); +} - elapsed_render = render_count - rps_ei-render_c0; - rps_ei-render_c0 = render_count; +static bool vlv_c0_above(struct drm_i915_private *dev_priv, +const struct intel_rps_ei *old, +const struct intel_rps_ei *now, +int threshold) +{ + u64 time, c0; - elapsed_media = media_count - rps_ei-media_c0; - rps_ei-media_c0 = media_count; + if (old-cz_clock == 0) + return false; - /* 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; + time = now-cz_clock - old-cz_clock; + time *= threshold * dev_priv-mem_freq; - /* -* Calculate overall C0 residency percentage -* only if elapsed time is non zero + /* Workload can be split between render + media, e.g. SwapBuffers +* being blitted in X after being rendered in mesa. To account for +* this we need to combine both engines into our activity counter. */ - if (elapsed_time) { - residency = - ((max(elapsed_render, elapsed_media) * 100) - / elapsed_time); - } + c0 = now-render_c0 - old-render_c0; + c0 += now-media_c0 - old-media_c0; + c0 *= 100 * VLV_CZ_CLOCK_TO_MILLI_SEC * 4 / 1000; - return residency; + return c0 = time; } -/** - * 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 int vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv) +void gen6_rps_reset_ei(struct drm_i915_private *dev_priv) { - u32 residency_C0_up = 0, residency_C0_down = 0; - int new_delay, adj; - - dev_priv-rps.ei_interrupt_count++; - - WARN_ON(!mutex_is_locked(dev_priv-rps.hw_lock)); + vlv_c0_read(dev_priv, dev_priv-rps.down_ei); + dev_priv-rps.up_ei = dev_priv-rps.down_ei; +
Re: [Intel-gfx] [PATCH 04/21] drm/i915: skylake scaler structure definitions
On Tue, Mar 17, 2015 at 09:20:11PM +, Konduru, Chandra wrote: -Original Message- From: Roper, Matthew D Sent: Tuesday, March 17, 2015 9:13 AM To: Konduru, Chandra Cc: intel-gfx@lists.freedesktop.org; Vetter, Daniel; Conselvan De Oliveira, Ander Subject: Re: [PATCH 04/21] drm/i915: skylake scaler structure definitions On Sat, Mar 14, 2015 at 10:55:29PM -0700, Chandra Konduru wrote: skylake scaler structure definitions. scalers live in crtc_state as they are pipe resources. They can be used either as plane scaler or panel fitter. scaler assigned to either plane (for plane scaling) or crtc (for panel fitting) is saved in scaler_id in plane_state or crtc_state respectively. scaler_id is used instead of scaler pointer in plane or crtc state to avoid updating scaler pointer everytime a new crtc_state is created. Signed-off-by: Chandra Konduru chandra.kond...@intel.com --- drivers/gpu/drm/i915/intel_drv.h | 99 ++ 1 file changed, 99 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 3f7d05e..d9a3b64 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -256,6 +256,35 @@ struct intel_plane_state { * enable/disable the primary plane */ bool hides_primary; + + /* + * scaler_id + *= -1 : not using a scaler + *= 0 : using a scalers + * + * plane requiring a scaler: + * - During check_plane, its bit is set in + * crtc_state-scaler_state.scaler_users by calling helper function + * update_scaler_users. + * - scaler_id indicates the scaler it got assigned. + * + * plane doesn't require a scaler: + * - this can happen when scaling is no more required or plane simply + * got disabled. + * - During check_plane, corresponding bit is reset in + * crtc_state-scaler_state.scaler_users by calling helper function + * update_scaler_users. + * + * There are two scenarios: + * 1. the freed up scaler is assigned to crtc or some other plane + * In this case, as part of plane programming scaler_id will be set + * to -1 using helper function detach_scalers + * 2. the freed up scaler is not assigned to anyone + * In this case, as part of plane programming scaler registers will + * be reset and scaler_id will also be reset to -1 using the same + * helper function detach_scalers + */ + int scaler_id; }; struct intel_initial_plane_config { @@ -265,6 +294,74 @@ struct intel_initial_plane_config { u32 base; }; +struct intel_scaler { + int id; + int in_use; + uint32_t mode; + uint32_t filter; + + /* + * Supported scaling ratio is represented as a range in [min max] + * variables. This range covers both up and downscaling + * where scaling ratio = (dst * 100)/src. + * In above range any value: + * 100 represents downscaling coverage + * 100 represents upscaling coverage + *= 100 represents no-scaling (i.e., 1:1) + * e.g., a min value = 50 means - supports upto 50% of original image + * a max value = 200 means - supports upto 200% of original image + * + * if incoming flip requires scaling in the supported [min max] range + * then requested scaling will be performed. + */ I've only skimmed a little ahead in the series, so I might have missed something, but do we really need to track these on a per-scaler basis? When you use the values here, I think you're always pulling the values from scaler[0] unconditionally from what I saw so duplicating the values for each scaler doesn't really help us. Is it possible to keep just one copy of these min/max values? On SKL all of our scalers are homogeneous, so it doesn't feel like there's too much value to duplicating these values. If we have a future platform with heterogeneous scalers, it seems like we can figure out how to handle that appropriately if/when we get there. I put them per scaler basis for future scalability, but can make them as one copy. It has some cascading effect on other patches so send out this one and other impacted ones. In my experience adding flexibility in the design for which we don't yet have a clear use-case established is a tricky design mistake: It always sounds good and very often turns out to be a costly endeavor. I much prefer to be as lazy as possible in code design and if there's no need yet to not write code. Also, can't we recompute these limits at runtime? I can't quickly check since the code is in some other patches, but iirc that would resolve some of the issues I've pointed out later on with keeping these in sync. I think as a design rule atomic state structures should be constrained to the
Re: [Intel-gfx] [PATCH 14/21] drm/i915: use current scaler state during readout_hw_state.
On Tue, Mar 17, 2015 at 06:54:30PM +, Konduru, Chandra wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, March 17, 2015 7:20 AM To: Konduru, Chandra Cc: intel-gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander; Vetter, Daniel Subject: Re: [Intel-gfx] [PATCH 14/21] drm/i915: use current scaler state during readout_hw_state. On Sat, Mar 14, 2015 at 10:55:39PM -0700, Chandra Konduru wrote: During readout_hw_state, whole pipe_config is built reading hw. But rebuilding scaler state from hw requires, - reading all planes and find its corresponding index in order to set its bits in scaler_users - reading cdclk and adjusted mode crtc clk in order to regenerate min scaling ratios. - some values directly from bspec To simplify things, for now using sw scaler state as readout state. Signed-off-by: Chandra Konduru chandra.kond...@intel.com At least the crtc scaler might get used by the bios for boot-up with non-native resolution. I do think we need to properly track those parts (and also double- check the hw state in pipe config compare function like we do for the old pfit state). To cover above case, will populate the crtc scaler state. But population of scalers used for planes is tricky, because in general there is no population of planes or its states from hw. From scaler point of this may be ok because there aren't any scalers in use for planes. Yeah plane scaler state is completely different. Imo it's ok to not care about that - as you spotted we almost completely lack state readout support for planes. No need to fix that in your series ;-) Maybe we need to always compute limits directly instead of storing the in the state structures? We are adjusting/computing some of limits from hw but not all limits. Along with crtc_scaler population, will add code to populate limits that can be computed from hw. Rest will carry from previous state. My suggestion was to completely remove the limit fields from the scaler state and always recompute them when needed. I think that would simplify the code in a few other places, and reduce changes that these computed values would get out of sync. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/21] drm/i915: Add helper function to update scaler_users in crtc_state
On Sun, Mar 15, 2015 at 6:55 AM, Chandra Konduru chandra.kond...@intel.com wrote: + /* +* check for rect size: +* min sizes in case of scaling involved +* max sizes in all cases +*/ + if ((need_scaling + (src_w scaler-min_src_w || src_h scaler-min_src_h || +dst_w scaler-min_dst_w || dst_h scaler-min_dst_h)) || + +src_w scaler-max_src_w || src_h scaler-max_src_h || +dst_w scaler-max_dst_w || dst_h scaler-max_dst_h) { Ok let's hope I've traced all the min/max stuff in your patches correctly. It looks like we only ever initialized them to fixed values, never changed them and only use them here. Spreading out the values like that without having a real need for this flexibility makes review really hard imo. What about instead adding a skl_check_scale_limits functions which does all these checks here and uses hardcoded values? That way you could move the commits about the various values (e.g. only 34% scaling and the other easier-to-understand limits) right next to the code that checks these limits? There's also some confusion with the overly generic (imo) old sprite code and its scaling limit checks. Imo we can look at that later on. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dp: return number of bytes written for short aux/i2c writes
On Tue, Mar 17, 2015 at 06:40:18PM +0200, Ville Syrjälä wrote: On Tue, Mar 17, 2015 at 05:18:54PM +0200, Jani Nikula wrote: Allow for a larger receive data size, and check if the receiver returned the number of bytes written. Without this, we've basically skipped all the unwritten bytes for short writes. Signed-off-by: Jani Nikula jani.nik...@intel.com Whoops :) Looks correct to me. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Queued for -next, thanks for the patch. -Daniel --- drivers/gpu/drm/i915/intel_dp.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5256c064da05..3967af10f53c 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -951,7 +951,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) case DP_AUX_NATIVE_WRITE: case DP_AUX_I2C_WRITE: txsize = msg-size ? HEADER_SIZE + msg-size : BARE_ADDRESS_SIZE; - rxsize = 1; + rxsize = 2; /* 0 or 1 data bytes */ if (WARN_ON(txsize 20)) return -E2BIG; @@ -962,8 +962,13 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) if (ret 0) { msg-reply = rxbuf[0] 4; - /* Return payload size. */ - ret = msg-size; + if (ret 1) { + /* Number of bytes written in a short write. */ + ret = clamp_t(int, rxbuf[1], 0, msg-size); + } else { + /* Return payload size. */ + ret = msg-size; + } } break; -- 2.1.4 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [patch] drm/i915: memory leak in __i915_gem_vma_create()
On Wed, Mar 18, 2015 at 10:36:45AM +0200, Jani Nikula wrote: On Wed, 18 Mar 2015, Dan Carpenter dan.carpen...@oracle.com wrote: In the original code then if WARN_ON(i915_is_ggtt(vm) != !!ggtt_view) was true then we leak vma. Presumably that doesn't happen often but static checkers complain and this bug is easy to fix. Fixes: c3bbb6f2825d ('drm/i915: Do not use ggtt_view with (aliasing) PPGTT') Signed-off-by: Dan Carpenter dan.carpen...@oracle.com Reviewed-by: Jani Nikula jani.nik...@intel.com Queued for -next, thanks for the patch. -Daniel diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index f1b9ea6..cbf013f 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2340,12 +2340,13 @@ __i915_gem_vma_create(struct drm_i915_gem_object *obj, struct i915_address_space *vm, const struct i915_ggtt_view *ggtt_view) { - struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL); - if (vma == NULL) - return ERR_PTR(-ENOMEM); + struct i915_vma *vma; if (WARN_ON(i915_is_ggtt(vm) != !!ggtt_view)) return ERR_PTR(-EINVAL); + vma = kzalloc(sizeof(*vma), GFP_KERNEL); + if (vma == NULL) + return ERR_PTR(-ENOMEM); INIT_LIST_HEAD(vma-vma_link); INIT_LIST_HEAD(vma-mm_list); -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: gen4: work around hang during hibernation
Imre Deak schreef op ma 02-03-2015 om 13:04 [+0200]: Bjørn reported that his machine hang during hibernation and eventually bisected the problem to the following commit: commit da2bc1b9db3351addd293e5b82757efe1f77ed1d Author: Imre Deak imre.d...@intel.com Date: Thu Oct 23 19:23:26 2014 +0300 drm/i915: add poweroff_late handler The problem seems to be that after the kernel puts the device into D3 the BIOS still tries to access it, or otherwise assumes that it's in D0. This is clearly bogus, since ACPI mandates that devices are put into D3 by the OSPM if they are not wake-up sources. In the future we want to unify more of the driver's runtime and system suspend paths, for example by skipping all the system suspend/hibernation hooks if the device is runtime suspended already. Accordingly for all other platforms the goal is still to properly power down the device during hibernation. v2: - Another GEN4 Lenovo laptop had the same issue, while platforms from other vendors (including mobile and desktop, GEN4 and non-GEN4) seem to work fine. Based on this apply the workaround on all GEN4 Lenovo platforms. - add code comment about failing platforms (Ville) The outdated ThinkPad X41 that I torture by running rc's showed identical symptoms, also since v3.19-rc1. It uses a gen3 chipset (it has a 915GM, I think, but I keep forgetting details like that). I did everything wrong to get this fixed (1: hope this gets magically fixed; 2: bisect it myself, thinking every now and then that I know better than git bisect which commit to choose; 3: finally grep lkml). So here I am late to the show. Reference: http://lists.freedesktop.org/archives/intel-gfx/2015-February/060633.html Reported-and-bisected-by: Bjørn Mork bj...@mork.no Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 30 +- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 4badb23..ff3662f 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -637,7 +637,7 @@ static int i915_drm_suspend(struct drm_device *dev) return 0; } -static int i915_drm_suspend_late(struct drm_device *drm_dev) +static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation) { struct drm_i915_private *dev_priv = drm_dev-dev_private; int ret; @@ -651,7 +651,17 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev) } pci_disable_device(drm_dev-pdev); - pci_set_power_state(drm_dev-pdev, PCI_D3hot); + /* + * During hibernation on some GEN4 platforms the BIOS may try to access + * the device even though it's already in D3 and hang the machine. So + * leave the device in D0 on those platforms and hope the BIOS will + * power down the device properly. Platforms where this was seen: + * Lenovo Thinkpad X301, X61s + */ + if (!(hibernation + drm_dev-pdev-subsystem_vendor == PCI_VENDOR_ID_LENOVO + INTEL_INFO(dev_priv)-gen == 4)) + pci_set_power_state(drm_dev-pdev, PCI_D3hot); return 0; } I'll paste a DRAFT patch that fixes this for that X41 at the end of the message. The patch is rather ugly. Should we perhaps try a quirk table or something like that? Paul Bolle 8 Subject: [PATCH] drm/i915: work around hang during hibernation on gen3 too Commit ab3be73fa7b4 (drm/i915: gen4: work around hang during hibernation) was targetted at gen4 platforms shipped by Lenovo. The same problem can also be seen on a Lenovo ThinkPad X41. Expand the test to catch that system too. Sadly, this system still uses IBM's subsystem vendor id. So we end up with a rather unpleasant test. Use the IS_GEN3() and IS_GEN4() macros to lessen the pain a bit. Not-yet-signed-off-by: Paul Bolle pebo...@tiscali.nl --- drivers/gpu/drm/i915/i915_drv.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index cc6ea53d2b81..3a07164f5860 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -641,11 +641,12 @@ static int i915_drm_suspend_late(struct drm_device *drm_dev, bool hibernation) * the device even though it's already in D3 and hang the machine. So * leave the device in D0 on those platforms and hope the BIOS will * power down the device properly. Platforms where this was seen: -* Lenovo Thinkpad X301, X61s +* Lenovo Thinkpad X301, X61s, X41 */ if (!(hibernation - drm_dev-pdev-subsystem_vendor == PCI_VENDOR_ID_LENOVO - INTEL_INFO(dev_priv)-gen == 4)) + (drm_dev-pdev-subsystem_vendor == PCI_VENDOR_ID_LENOVO || + drm_dev-pdev-subsystem_vendor == PCI_SUBVENDOR_ID_IBM) +
Re: [Intel-gfx] [PATCH 04/12] drm/i915/bdw: Add ppgtt info for dynamic pages
On 3/3/2015 12:23 PM, akash goel wrote: On Fri, Feb 20, 2015 at 11:15 PM, Michel Thierry michel.thie...@intel.com wrote: From: Ben Widawskybenjamin.widaw...@intel.com Note that there is no gen8 ppgtt debug_dump function yet. Signed-off-by: Ben Widawskyb...@bwidawsk.net Signed-off-by: Michel Thierrymichel.thie...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 19 ++- drivers/gpu/drm/i915/i915_gem_gtt.c | 32 drivers/gpu/drm/i915/i915_gem_gtt.h | 9 + 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 40630bd..93c34ab 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2165,7 +2165,6 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; struct intel_engine_cs *ring; - struct drm_file *file; int i; if (INTEL_INFO(dev)-gen == 6) @@ -2189,14 +2188,6 @@ static void gen6_ppgtt_info(struct seq_file *m, struct drm_device *dev) ppgtt-debug_dump(ppgtt, m); } - - list_for_each_entry_reverse(file, dev-filelist, lhead) { - struct drm_i915_file_private *file_priv = file-driver_priv; - - seq_printf(m, proc: %s\n, - get_pid_task(file-pid, PIDTYPE_PID)-comm); - idr_for_each(file_priv-context_idr, per_file_ctx, m); - } } static int i915_ppgtt_info(struct seq_file *m, void *data) @@ -2204,6 +2195,7 @@ static int i915_ppgtt_info(struct seq_file *m, void *data) struct drm_info_node *node = m-private; struct drm_device *dev = node-minor-dev; struct drm_i915_private *dev_priv = dev-dev_private; + struct drm_file *file; int ret = mutex_lock_interruptible(dev-struct_mutex); if (ret) @@ -2215,6 +2207,15 @@ static int i915_ppgtt_info(struct seq_file *m, void *data) else if (INTEL_INFO(dev)-gen = 6) gen6_ppgtt_info(m, dev); + list_for_each_entry_reverse(file, dev-filelist, lhead) { + struct drm_i915_file_private *file_priv = file-driver_priv; + + seq_printf(m, \nproc: %s\n, + get_pid_task(file-pid, PIDTYPE_PID)-comm); + idr_for_each(file_priv-context_idr, per_file_ctx, +(void *)(unsigned long)m); + } + intel_runtime_pm_put(dev_priv); mutex_unlock(dev-struct_mutex); diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index ecfb62a..1edcc17 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2125,6 +2125,38 @@ static void gen8_ggtt_clear_range(struct i915_address_space *vm, readl(gtt_base); } +void gen8_for_every_pdpe_pde(struct i915_hw_ppgtt *ppgtt, +void (*callback)(struct i915_page_directory_pointer_entry *pdp, + struct i915_page_directory_entry *pd, + struct i915_page_table_entry *pt, + unsigned pdpe, + unsigned pde, + void *data), +void *data) +{ + uint64_t start = ppgtt-base.start; + uint64_t length = ppgtt-base.total; + uint64_t pdpe, pde, temp; + + struct i915_page_directory_entry *pd; + struct i915_page_table_entry *pt; + + gen8_for_each_pdpe(pd, ppgtt-pdp, start, length, temp, pdpe) { + uint64_t pd_start = start, pd_length = length; + int i; + + if (pd == NULL) { + for (i = 0; i GEN8_PDES_PER_PAGE; i++) + callback(ppgtt-pdp, NULL, NULL, pdpe, i, data); + continue; + } + + gen8_for_each_pde(pt, pd, pd_start, pd_length, temp, pde) { + callback(ppgtt-pdp, pd, pt, pdpe, pde, data); + } + } +} + static void gen6_ggtt_clear_range(struct i915_address_space *vm, uint64_t start, uint64_t length, diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index a33c6e9..144858e 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -483,6 +483,15 @@ static inline size_t gen8_pde_count(uint64_t addr, uint64_t length) return i915_pde_index(end, GEN8_PDE_SHIFT) - i915_pde_index(addr, GEN8_PDE_SHIFT); } +void gen8_for_every_pdpe_pde(struct i915_hw_ppgtt *ppgtt, +void (*callback)(struct i915_page_directory_pointer_entry *pdp, +
Re: [Intel-gfx] [PATCH] drm/i915: Compare GGTT view structs instead of types
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5981 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -1 268/268 267/268 ILK 303/303 303/303 SNB 283/283 283/283 IVB 343/343 343/343 BYT 287/287 287/287 HSW -1 363/363 362/363 BDW 308/308 308/308 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt_gem_userptr_blits_minor-normal-sync PASS(3) DMESG_WARN(1)PASS(1) *HSW igt_gem_storedw_loop_blt PASS(2) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 7/7] drm/i915: Boost GPU frequency if we detect outstanding pageflips
On Wednesday 18 March 2015 03:18 PM, Chris Wilson wrote: If we hit a vblank and see that have a pageflip queue but not yet processed, ensure that the GPU is running at maximum in order to clear the backlog. Pageflips are only queued for the following vblank, if we miss it, there will be a visible stutter. Boosting the GPU frequency doesn't prevent us from missing the target vblank, but it should help the subsequent frames hitting theirs. v2: Reorder vblank vs flip-complete so that we only check for a missed flip after processing the completion events, and avoid spurious boosts. v3: Rename missed_vblank v4: Rebase v5: Cancel the outstanding work in runtime suspend v6: Rebase v7: Rebase required fixing Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Daniel Vetter daniel.vet...@ffwll.ch Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Deepak Sdeepa...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 11 --- drivers/gpu/drm/i915/intel_drv.h | 2 ++ drivers/gpu/drm/i915/intel_pm.c | 35 +++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f1c0295f69e5..0efb19a9b9a5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9852,6 +9852,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe) struct drm_i915_private *dev_priv = dev-dev_private; struct drm_crtc *crtc = dev_priv-pipe_to_crtc_mapping[pipe]; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_unpin_work *work; WARN_ON(!in_interrupt()); @@ -9859,12 +9860,16 @@ void intel_check_page_flip(struct drm_device *dev, int pipe) return; spin_lock(dev-event_lock); - if (intel_crtc-unpin_work __intel_pageflip_stall_check(dev, crtc)) { + work = intel_crtc-unpin_work; + if (work != NULL __intel_pageflip_stall_check(dev, crtc)) { WARN_ONCE(1, Kicking stuck page flip: queued at %d, now %d\n, -intel_crtc-unpin_work-flip_queued_vblank, -drm_vblank_count(dev, pipe)); +work-flip_queued_vblank, drm_vblank_count(dev, pipe)); page_flip_completed(intel_crtc); + work = NULL; } + if (work != NULL + drm_vblank_count(dev, pipe) - work-flip_queued_vblank 1) + intel_queue_rps_boost_for_request(dev, work-flip_queued_req); spin_unlock(dev-event_lock); } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 8bb18e507f5f..d6e7ac8c2284 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1246,6 +1246,8 @@ void gen6_rps_busy(struct drm_i915_private *dev_priv); void gen6_rps_reset_ei(struct drm_i915_private *dev_priv); void gen6_rps_idle(struct drm_i915_private *dev_priv); void gen6_rps_boost(struct drm_i915_private *dev_priv); +void intel_queue_rps_boost_for_request(struct drm_device *dev, + struct drm_i915_gem_request *rq); void ilk_wm_get_hw_state(struct drm_device *dev); void skl_wm_get_hw_state(struct drm_device *dev); void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv, diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8b16bb3ae09f..e8111be32ed0 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6751,6 +6751,41 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val) return val / GT_FREQUENCY_MULTIPLIER; } +struct request_boost { + struct work_struct work; + struct drm_i915_gem_request *rq; +}; + +static void __intel_rps_boost_work(struct work_struct *work) +{ + struct request_boost *boost = container_of(work, struct request_boost, work); + + if (!i915_gem_request_completed(boost-rq, true)) + gen6_rps_boost(to_i915(boost-rq-ring-dev)); + + i915_gem_request_unreference__unlocked(boost-rq); + kfree(boost); +} + +void intel_queue_rps_boost_for_request(struct drm_device *dev, + struct drm_i915_gem_request *rq) +{ + struct request_boost *boost; + + if (rq == NULL || INTEL_INFO(dev)-gen 6) + return; + + boost = kmalloc(sizeof(*boost), GFP_ATOMIC); + if (boost == NULL) + return; + + i915_gem_request_reference(rq); + boost-rq = rq; + + INIT_WORK(boost-work, __intel_rps_boost_work); + queue_work(to_i915(dev)-wq, boost-work); +} + void intel_pm_setup(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; Patch looks fine Reviewed-by: Deepak Sdeepa...@linux.intel.com ___ Intel-gfx mailing list
Re: [Intel-gfx] [PATCH] tests/gem_exec_lut_handle
On Wed, Mar 18, 2015 at 11:19:32AM +0100, Daniel Vetter wrote: Reduce default number of repeats a lot. High repeat count is only useful for microbenchmarking, not that much for regression testing. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87131 This just to hide the regression that exec got a lot slower? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: kerneldoc for i915_gem_shrinker.c
And remove one bogus * from i915_gem_gtt.c since that's not a kerneldoc there. Signed-off-by: Daniel Vetter daniel.vet...@intel.com --- Documentation/DocBook/drm.tmpl | 13 +++- drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- drivers/gpu/drm/i915/i915_gem_shrinker.c | 34 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 7a45775518f6..f4976cd7b32b 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -4184,7 +4184,7 @@ int num_ioctls;/synopsis sect2 titleBuffer Object Eviction/title para - This section documents the interface function for evicting buffer + This section documents the interface functions for evicting buffer objects to make space available in the virtual gpu address spaces. Note that this is mostly orthogonal to shrinking buffer objects caches, which has the goal to make main memory (shared with the gpu @@ -4192,6 +4192,17 @@ int num_ioctls;/synopsis /para !Idrivers/gpu/drm/i915/i915_gem_evict.c /sect2 + sect2 +titleBuffer Object Memory Shrinking/title + para + This section documents the interface function for shrinking memory + usage of buffer object caches. Shrinking is used to make main memory + available. Note that this is mostly orthogonal to evicting buffer + objects, which has the goal to make space in gpu virtual address + spaces. + /para +!Idrivers/gpu/drm/i915/i915_gem_shrinker.c + /sect2 /sect1 sect1 diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index cbf013fd6b98..d8ff1a8e9d43 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -698,7 +698,7 @@ static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt, return 0; } -/** +/* * GEN8 legacy ppgtt programming is accomplished through a max 4 PDP registers * with a net effect resembling a 2-level page table in normal x86 terms. Each * PDP represents 1GB of memory 4 * 512 * 512 * 4096 = 4GB legacy 32b address diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c index 9ac78b3d6899..86102e5f3a0a 100644 --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c @@ -47,6 +47,20 @@ static bool mutex_is_locked_by(struct mutex *mutex, struct task_struct *task) #endif } +/** + * i915_gem_shrink - Shrink buffer object caches + * @dev_priv: i915 device + * @target: memory space to make available, in pages + * @flags: control flags for selecting cache types + * + * This function is the main interface to the shrinker. It will try to release + * up to @target pages of main memory backing storage from buffer objects. + * Selection of the specific caches can be done with @flags. This is e.g. useful + * when purgeable objects should be removed from caches preferentially. + * + * Returns: + * The number of pages of backing storage actually released. + */ unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv, long target, unsigned flags) @@ -118,6 +132,20 @@ i915_gem_shrink(struct drm_i915_private *dev_priv, return count; } +/** + * i915_gem_shrink - Shrink buffer object caches completely + * @dev_priv: i915 device + * + * This is a simple wraper around i915_gem_shrink() to aggressively shrink all + * caches completely. It also first waits for and retires all outstanding + * requests to also be able to release backing storage for active objects. + * + * This should only be used in code to intentionally quiescent the gpu or as a + * last-ditch effort when memory seems to have run out. + * + * Returns: + * The number of pages of backing storage actually released. + */ unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv) { i915_gem_evict_everything(dev_priv-dev); @@ -279,6 +307,12 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr) return NOTIFY_DONE; } +/** + * i915_gem_shrinker_init - Initialize i915 shrinker + * @dev_priv: i915 device + * + * This function registers and sets up the i915 shrinker and OOM handler. + */ void i915_gem_shrinker_init(struct drm_i915_private *dev_priv) { dev_priv-mm.shrinker.scan_objects = i915_gem_shrinker_scan; -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Extract i915_gem_shrinker.c
Two code changes: - Extract i915_gem_shrinker_init. - Inline i915_gem_object_is_purgeable since we open-code it everywhere else too. This already has the benefit of pulling all the shrinker code together, next patch adds a bit of kerneldoc. Signed-off-by: Daniel Vetter daniel.vet...@intel.com --- drivers/gpu/drm/i915/Makefile| 1 + drivers/gpu/drm/i915/i915_drv.h | 17 +- drivers/gpu/drm/i915/i915_gem.c | 274 + drivers/gpu/drm/i915/i915_gem_shrinker.c | 291 +++ 4 files changed, 306 insertions(+), 277 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_gem_shrinker.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index d3ebaf204408..a69002e2257d 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -28,6 +28,7 @@ i915-y += i915_cmd_parser.o \ i915_gem_execbuffer.o \ i915_gem_gtt.o \ i915_gem.o \ + i915_gem_shrinker.o \ i915_gem_stolen.o \ i915_gem_tiling.o \ i915_gem_userptr.o \ diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 81f60b48def2..adf95d4c1afc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2601,12 +2601,6 @@ int i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data, int i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); void i915_gem_load(struct drm_device *dev); -unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv, - long target, - unsigned flags); -#define I915_SHRINK_PURGEABLE 0x1 -#define I915_SHRINK_UNBOUND 0x2 -#define I915_SHRINK_BOUND 0x4 void *i915_gem_object_alloc(struct drm_device *dev); void i915_gem_object_free(struct drm_i915_gem_object *obj); void i915_gem_object_init(struct drm_i915_gem_object *obj, @@ -2954,6 +2948,17 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev, u32 gtt_offset, u32 size); +/* i915_gem_shrinker.c */ +unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv, + long target, + unsigned flags); +#define I915_SHRINK_PURGEABLE 0x1 +#define I915_SHRINK_UNBOUND 0x2 +#define I915_SHRINK_BOUND 0x4 +unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv); +void i915_gem_shrinker_init(struct drm_i915_private *dev_priv); + + /* i915_gem_tiling.c */ static inline bool i915_gem_object_needs_bit17_swizzle(struct drm_i915_gem_object *obj) { diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 70de915cec11..8a8e4d188d2f 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1,5 +1,5 @@ /* - * Copyright © 2008 Intel Corporation + * Copyright © 2008-2015 Intel Corporation * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the Software), @@ -32,7 +32,6 @@ #include i915_vgpu.h #include i915_trace.h #include intel_drv.h -#include linux/oom.h #include linux/shmem_fs.h #include linux/slab.h #include linux/swap.h @@ -53,15 +52,6 @@ static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj, struct drm_i915_fence_reg *fence, bool enable); -static unsigned long i915_gem_shrinker_count(struct shrinker *shrinker, -struct shrink_control *sc); -static unsigned long i915_gem_shrinker_scan(struct shrinker *shrinker, - struct shrink_control *sc); -static int i915_gem_shrinker_oom(struct notifier_block *nb, -unsigned long event, -void *ptr); -static unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv); - static bool cpu_cache_is_coherent(struct drm_device *dev, enum i915_cache_level level) { @@ -1936,12 +1926,6 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data, return i915_gem_mmap_gtt(file, dev, args-handle, args-offset); } -static inline int -i915_gem_object_is_purgeable(struct drm_i915_gem_object *obj) -{ - return obj-madv == I915_MADV_DONTNEED; -} - /* Immediately discard the backing storage */ static void i915_gem_object_truncate(struct drm_i915_gem_object *obj) @@ -2047,85 +2031,6 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj) return 0; } -unsigned long -i915_gem_shrink(struct drm_i915_private *dev_priv, - long target, unsigned flags) -{ - const struct { - struct list_head *list; - unsigned int
Re: [Intel-gfx] [patch] drm/i915: memory leak in __i915_gem_vma_create()
On ke, 2015-03-18 at 09:41 +0100, Daniel Vetter wrote: On Wed, Mar 18, 2015 at 10:36:45AM +0200, Jani Nikula wrote: On Wed, 18 Mar 2015, Dan Carpenter dan.carpen...@oracle.com wrote: In the original code then if WARN_ON(i915_is_ggtt(vm) != !!ggtt_view) was true then we leak vma. Presumably that doesn't happen often but static checkers complain and this bug is easy to fix. Correct, it was originally BUG_ON and then relaxed down to a warning in last series, so it slipped through. So it should practically be impossible to happen, but suits well to make static checker happier. Regards, Joonas Fixes: c3bbb6f2825d ('drm/i915: Do not use ggtt_view with (aliasing) PPGTT') Signed-off-by: Dan Carpenter dan.carpen...@oracle.com Reviewed-by: Jani Nikula jani.nik...@intel.com Queued for -next, thanks for the patch. -Daniel diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index f1b9ea6..cbf013f 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -2340,12 +2340,13 @@ __i915_gem_vma_create(struct drm_i915_gem_object *obj, struct i915_address_space *vm, const struct i915_ggtt_view *ggtt_view) { - struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL); - if (vma == NULL) - return ERR_PTR(-ENOMEM); + struct i915_vma *vma; if (WARN_ON(i915_is_ggtt(vm) != !!ggtt_view)) return ERR_PTR(-EINVAL); + vma = kzalloc(sizeof(*vma), GFP_KERNEL); + if (vma == NULL) + return ERR_PTR(-ENOMEM); INIT_LIST_HEAD(vma-vma_link); INIT_LIST_HEAD(vma-mm_list); -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/7] drm/i915/skl: Extract tile height code into a helper function
On ti, 2015-03-17 at 15:45 +, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com It will be used in a later patch and also convert all height parameters from int to unsigned int. v2: Rebased for fb modifiers. v3: Fixed v2 rebase. v4: * Height should be unsigned int. * Make it take pixel_format for consistency and simplicity. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Reviewed-by: Michel Thierry michel.thie...@intel.com (v1) Reviewed-by: Joonas Lahtinen joonas.lahti...@linux.intel.com (v4) --- drivers/gpu/drm/i915/intel_display.c | 43 +--- drivers/gpu/drm/i915/intel_drv.h | 7 +++--- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 90b460c..a307979 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2194,13 +2194,12 @@ static bool need_vtd_wa(struct drm_device *dev) return false; } -int -intel_fb_align_height(struct drm_device *dev, int height, - uint32_t pixel_format, - uint64_t fb_format_modifier) +static unsigned int +intel_tile_height(struct drm_device *dev, uint32_t pixel_format, + uint64_t fb_format_modifier) { - int tile_height; - uint32_t bits_per_pixel; + unsigned int tile_height; + uint32_t pixel_bytes; switch (fb_format_modifier) { case DRM_FORMAT_MOD_NONE: @@ -2213,20 +2212,20 @@ intel_fb_align_height(struct drm_device *dev, int height, tile_height = 32; break; case I915_FORMAT_MOD_Yf_TILED: - bits_per_pixel = drm_format_plane_cpp(pixel_format, 0) * 8; - switch (bits_per_pixel) { + pixel_bytes = drm_format_plane_cpp(pixel_format, 0); + switch (pixel_bytes) { default: - case 8: + case 1: tile_height = 64; break; - case 16: - case 32: + case 2: + case 4: tile_height = 32; break; - case 64: + case 8: tile_height = 16; break; - case 128: + case 16: WARN_ONCE(1, 128-bit pixels are not supported for display!); tile_height = 16; @@ -2239,7 +2238,15 @@ intel_fb_align_height(struct drm_device *dev, int height, break; } - return ALIGN(height, tile_height); + return tile_height; +} + +unsigned int +intel_fb_align_height(struct drm_device *dev, unsigned int height, + uint32_t pixel_format, uint64_t fb_format_modifier) +{ + return ALIGN(height, intel_tile_height(dev, pixel_format, +fb_format_modifier)); } int @@ -6772,7 +6779,7 @@ i9xx_get_initial_plane_config(struct intel_crtc *crtc, u32 val, base, offset; int pipe = crtc-pipe, plane = crtc-plane; int fourcc, pixel_format; - int aligned_height; + unsigned int aligned_height; struct drm_framebuffer *fb; struct intel_framebuffer *intel_fb; @@ -7810,7 +7817,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc, u32 val, base, offset, stride_mult, tiling; int pipe = crtc-pipe; int fourcc, pixel_format; - int aligned_height; + unsigned int aligned_height; struct drm_framebuffer *fb; struct intel_framebuffer *intel_fb; @@ -7918,7 +7925,7 @@ ironlake_get_initial_plane_config(struct intel_crtc *crtc, u32 val, base, offset; int pipe = crtc-pipe; int fourcc, pixel_format; - int aligned_height; + unsigned int aligned_height; struct drm_framebuffer *fb; struct intel_framebuffer *intel_fb; @@ -12849,7 +12856,7 @@ static int intel_framebuffer_init(struct drm_device *dev, struct drm_mode_fb_cmd2 *mode_cmd, struct drm_i915_gem_object *obj) { - int aligned_height; + unsigned int aligned_height; int ret; u32 pitch_limit, stride_alignment; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index a1baaa1..5254540 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -904,9 +904,10 @@ void intel_frontbuffer_flip(struct drm_device *dev, intel_frontbuffer_flush(dev, frontbuffer_bits); } -int intel_fb_align_height(struct drm_device *dev, int height, - uint32_t pixel_format, - uint64_t fb_format_modifier); +unsigned int intel_fb_align_height(struct drm_device *dev, +
Re: [Intel-gfx] [PATCH v2 4/7] drm/i915: Agressive downclocking on Baytrail
On Wednesday 18 March 2015 04:53 PM, Chris Wilson wrote: On Wed, Mar 18, 2015 at 04:45:08PM +0530, Deepak S wrote: + if (val != dev_priv-rps.cur_freq) { vlv_punit_write(dev_priv, PUNIT_REG_GPU_FREQ_REQ, val); + gen6_set_rps_thresholds(dev_priv, val); I think gen6_set_rps_thresholds should be under baytrail specific with platform check? The only difference for cherryview is that it doesn't use GEN6_RP_MEDIA_TURBO. Was that intentional? The whole idea is that RPS should be autotuning for different workloads, but those metrics are equivalent across GPU. -Chris Atleast based on the spec GEN6_RP_MEDIA_TURBO left out of CHV. I am yet to look at the latest Spec. Also, Most of RP register for CHV falls under Comman well, I hope re-adjusting the rps_threshold will not causing power issues since we have to wakeup both RENDER/MEDIA to access the register -Deepak ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/dp: Use I2C_WRITE_STATUS_UPDATE to drain partial I2C_WRITE requests
On Fri, 13 Mar 2015, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When an i2c WRITE gets an i2c defer or short i2c ack reply, we are supposed to switch the request from I2C_WRITE to I2C_WRITE_STATUS_UPDATE when we continue to poll for the completion of the request. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/drm_dp_helper.c | 41 + 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index d5368ea..4db81a6 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -422,6 +422,25 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter *adapter) I2C_FUNC_10BIT_ADDR; } +static void drm_dp_i2c_msg_set_request(struct drm_dp_aux_msg *msg, +const struct i2c_msg *i2c_msg) +{ + msg-request = (i2c_msg-flags I2C_M_RD) ? + DP_AUX_I2C_READ : DP_AUX_I2C_WRITE; + msg-request |= DP_AUX_I2C_MOT; +} + +static void drm_dp_i2c_msg_write_status_update(struct drm_dp_aux_msg *msg) +{ + /* + * In case of i2c defer or short i2c ack reply to a write, + * we need to switch to WRITE_STATUS_UPDATE to drain the + * rest of the message + */ + if ((msg-request ~DP_AUX_I2C_MOT) == DP_AUX_I2C_WRITE) + msg-request |= DP_AUX_I2C_WRITE_STATUS_UPDATE; This only works because DP_AUX_I2C_WRITE == 0, and it may not be obvious to the casual reader that DP_AUX_I2C_WRITE_STATUS_UPDATE is a replacement not an addition to DP_AUX_I2C_WRITE. Whether you fix that or not, this is Reviewed-by: Jani Nikula jani.nik...@intel.com +} + /* * Transfer a single I2C-over-AUX message and handle various error conditions, * retrying the transaction as appropriate. It is assumed that the @@ -490,6 +509,8 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) * Both native ACK and I2C ACK replies received. We * can assume the transfer was successful. */ + if (ret != msg-size) + drm_dp_i2c_msg_write_status_update(msg); return ret; case DP_AUX_I2C_REPLY_NACK: @@ -501,6 +522,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) DRM_DEBUG_KMS(I2C defer\n); aux-i2c_defer_count++; usleep_range(400, 500); + drm_dp_i2c_msg_write_status_update(msg); continue; default: @@ -566,10 +588,7 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, for (i = 0; i num; i++) { msg.address = msgs[i].addr; - msg.request = (msgs[i].flags I2C_M_RD) ? - DP_AUX_I2C_READ : - DP_AUX_I2C_WRITE; - msg.request |= DP_AUX_I2C_MOT; + drm_dp_i2c_msg_set_request(msg, msgs[i]); /* Send a bare address packet to start the transaction. * Zero sized messages specify an address only (bare * address) transaction. @@ -577,6 +596,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, msg.buffer = NULL; msg.size = 0; err = drm_dp_i2c_do_msg(aux, msg); + + /* + * Reset msg.request in case in case it got + * changed into a WRITE_STATUS_UPDATE. + */ + drm_dp_i2c_msg_set_request(msg, msgs[i]); + if (err 0) break; /* We want each transaction to be as large as possible, but @@ -589,6 +615,13 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, msg.size = min(transfer_size, msgs[i].len - j); err = drm_dp_i2c_drain_msg(aux, msg); + + /* + * Reset msg.request in case in case it got + * changed into a WRITE_STATUS_UPDATE. + */ + drm_dp_i2c_msg_set_request(msg, msgs[i]); + if (err 0) break; transfer_size = err; -- 2.0.5 ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: Use the CRC gpio for panel enable/disable
On Mon, Mar 16, 2015 at 5:42 AM, Shobhit Kumar shobhit.ku...@intel.com wrote: The CRC (Crystal Cove) PMIC, controls the panel enable and disable signals for BYT for dsi panels. This is indicated in the VBT fields. Use that to initialize and use GPIO based control for these signals. v2: Use the newer gpiod interface(Alexandre) v3: Remove the redundant checks and unused code (Ville) CC: Samuel Ortiz sa...@linux.intel.com Cc: Linus Walleij linus.wall...@linaro.org Cc: Alexandre Courbot gnu...@gmail.com Cc: Thierry Reding thierry.red...@gmail.com Signed-off-by: Shobhit Kumar shobhit.ku...@intel.com NACK. This is not a GPIO but a special-purpose register as can be seen from other discussions. Use a fixed voltage regulator spun from an MFD cell instead of shoehorning this into GPIO. Yours, Linus Walleij ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Use GGTT view when (un)pinning objects to planes
On ti, 2015-03-17 at 15:45 +, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com To support frame buffer rotation we need to be able to pass on the information on what kind of GGTT view is required for display. This patch just adds the parameter and makes all the callers default to the normal view. v2: Rebased for ggtt view changes. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Cc: Joonas Lahtinen joonas.lahti...@linux.intel.com --- drivers/gpu/drm/i915/i915_drv.h | 14 +++--- drivers/gpu/drm/i915/i915_gem.c | 22 ++ drivers/gpu/drm/i915/intel_display.c | 7 --- drivers/gpu/drm/i915/intel_overlay.c | 3 ++- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 81f60b4..19b9e69 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2772,8 +2772,10 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write); int __must_check i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, u32 alignment, - struct intel_engine_cs *pipelined); -void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object *obj); + struct intel_engine_cs *pipelined, + const struct i915_ggtt_view *view); +void i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object *obj, + const struct i915_ggtt_view *view); int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, int align); int i915_gem_open(struct drm_device *dev, struct drm_file *file); @@ -2882,7 +2884,13 @@ i915_gem_object_ggtt_unbind(struct drm_i915_gem_object *obj) return i915_vma_unbind(i915_gem_obj_to_ggtt(obj)); } -void i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj); +void i915_gem_object_ggtt_unpin_view(struct drm_i915_gem_object *obj, + const struct i915_ggtt_view *view); +static inline void +i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj) +{ + i915_gem_object_ggtt_unpin_view(obj, i915_ggtt_view_normal); +} /* i915_gem_context.c */ int __must_check i915_gem_context_init(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 533ef37..58723a3 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3957,7 +3957,8 @@ static bool is_pin_display(struct drm_i915_gem_object *obj) int i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, u32 alignment, - struct intel_engine_cs *pipelined) + struct intel_engine_cs *pipelined, + const struct i915_ggtt_view *view) { u32 old_read_domains, old_write_domain; bool was_pin_display; @@ -3993,7 +3994,9 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, * (e.g. libkms for the bootup splash), we have to ensure that we * always use map_and_fenceable for all scanout buffers. */ - ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE); + ret = i915_gem_object_ggtt_pin(obj, view, alignment, +view-type == I915_GGTT_VIEW_NORMAL ? +PIN_MAPPABLE : 0); I'm slightly concerned about making an assumption that other but normal views need not to be mappable (when none are defined). As discussed in IRC, this should be moved later into the series when we actually know about the other views. if (ret) goto err_unpin_display; @@ -4021,9 +4024,11 @@ err_unpin_display: } void -i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object *obj) +i915_gem_object_unpin_from_display_plane(struct drm_i915_gem_object *obj, + const struct i915_ggtt_view *view) { - i915_gem_object_ggtt_unpin(obj); + i915_gem_object_ggtt_unpin_view(obj, view); + obj-pin_display = is_pin_display(obj); } @@ -4296,15 +4301,16 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj, } void -i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj) +i915_gem_object_ggtt_unpin_view(struct drm_i915_gem_object *obj, + const struct i915_ggtt_view *view) { - struct i915_vma *vma = i915_gem_obj_to_ggtt(obj); + struct i915_vma *vma = i915_gem_obj_to_ggtt_view(obj, view); BUG_ON(!vma); BUG_ON(vma-pin_count == 0); - BUG_ON(!i915_gem_obj_ggtt_bound(obj)); + BUG_ON(!i915_gem_obj_ggtt_bound_view(obj, view-type)); - if
Re: [Intel-gfx] [PATCH 1/2] gpio/crystalcove: Export Panel and backlight en/disable signals as GPIO
On Wed, Mar 18, 2015 at 12:50:51PM +0100, Linus Walleij wrote: On Thu, Mar 12, 2015 at 4:06 PM, Kumar, Shobhit shobhit.ku...@intel.com wrote: On Mon, 2015-03-09 at 18:15 +0100, Linus Walleij wrote: On Fri, Mar 6, 2015 at 5:23 PM, Kumar, Shobhit shobhit.ku...@intel.com wrote: There are actually two lines for Panel Power control and Backlight enable/disable. I have already moved towards adding a new Cell device for PWM child device and a new pwm driver for the same. That will take care of backlight thingy. But for the Panel power control, I am at loss for how best to program that. Isn't it just a very simple regulator (just on/off of fixed voltage) cell? It is just behaving as an output GPIO line and for that reason I feel that rather than adding a new regulator driver, I disagree. GPIO reads general purpose input/output. This is not a GPIO, this is a special purpose thing and IMO it should be modeled directly as a regulator. The idea behind reusing gpio was that we can reuse the dynamic lookup stuff so that i915 can find the gpio. And it is a gpio (there's lots more on that chip), it's just that intel tends to hand you recommended board layouts. And there's different ones so only really i915.ko can tell you if it's indeed used to control the panel or not (after consulting a bunch of vbios tables). Regulator seems way too overkill for this, especially since it tends to be a lie on systems where the panel is not connected to that gpio line. And afaiui the point of the regulator subsystem is that you just ask for the regulator for your IP block and then magically get handed the right bit (or a dummy one). This is very much not the case, hw descriptions on x86 in this area are a kludge worse than board tables since we can't even fix up what the bios hands us anywhere. So for me plan B is to just handroll our own thing using the component framework when reusing gpios isn't acceptable. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: kerneldoc for i915_gem_shrinker.c
And remove one bogus * from i915_gem_gtt.c since that's not a kerneldoc there. v2: Review from Chris: - Clarify memory space to better distinguish from address space. - Add note that shrink doesn't guarantee the freed memory and that users must fall back to shrink_all. - Explain how pinning ties in with eviction/shrinker. Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Daniel Vetter daniel.vet...@intel.com --- Documentation/DocBook/drm.tmpl | 13 +- drivers/gpu/drm/i915/i915_gem_evict.c| 4 +++ drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- drivers/gpu/drm/i915/i915_gem_shrinker.c | 44 4 files changed, 61 insertions(+), 2 deletions(-) diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 7a45775518f6..f4976cd7b32b 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -4184,7 +4184,7 @@ int num_ioctls;/synopsis sect2 titleBuffer Object Eviction/title para - This section documents the interface function for evicting buffer + This section documents the interface functions for evicting buffer objects to make space available in the virtual gpu address spaces. Note that this is mostly orthogonal to shrinking buffer objects caches, which has the goal to make main memory (shared with the gpu @@ -4192,6 +4192,17 @@ int num_ioctls;/synopsis /para !Idrivers/gpu/drm/i915/i915_gem_evict.c /sect2 + sect2 +titleBuffer Object Memory Shrinking/title + para + This section documents the interface function for shrinking memory + usage of buffer object caches. Shrinking is used to make main memory + available. Note that this is mostly orthogonal to evicting buffer + objects, which has the goal to make space in gpu virtual address + spaces. + /para +!Idrivers/gpu/drm/i915/i915_gem_shrinker.c + /sect2 /sect1 sect1 diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c index e3a49d94da3a..d09e35ed9c9a 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -63,6 +63,10 @@ mark_free(struct i915_vma *vma, struct list_head *unwind) * * This function is used by the object/vma binding code. * + * Since this function is only used to free up virtual address space it only + * ignores pinned vmas, and not object where the backing storage itself is + * pinned. Hence obj-pages_pin_count does not protect against eviction. + * * To clarify: This is for freeing up virtual address space, not for freeing * memory in e.g. the shrinker. */ diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index cbf013fd6b98..d8ff1a8e9d43 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -698,7 +698,7 @@ static int gen8_ppgtt_setup_page_tables(struct i915_hw_ppgtt *ppgtt, return 0; } -/** +/* * GEN8 legacy ppgtt programming is accomplished through a max 4 PDP registers * with a net effect resembling a 2-level page table in normal x86 terms. Each * PDP represents 1GB of memory 4 * 512 * 512 * 4096 = 4GB legacy 32b address diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c index 9ac78b3d6899..f7929e769250 100644 --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c @@ -47,6 +47,30 @@ static bool mutex_is_locked_by(struct mutex *mutex, struct task_struct *task) #endif } +/** + * i915_gem_shrink - Shrink buffer object caches + * @dev_priv: i915 device + * @target: amount of memory to make available, in pages + * @flags: control flags for selecting cache types + * + * This function is the main interface to the shrinker. It will try to release + * up to @target pages of main memory backing storage from buffer objects. + * Selection of the specific caches can be done with @flags. This is e.g. useful + * when purgeable objects should be removed from caches preferentially. + * + * Note that it's not guaranteed that released amount is actually available as + * free system memory - the pages might still be in-used to due to other reasons + * (like cpu mmaps) or the mm core has reused them before we could grab them. + * Therefore code that needs to explicitly shrink buffer objects caches (e.g. to + * avoid deadlocks in memory reclaim) must fall back to i915_gem_shrink_all(). + * + * Also note that any kind of pinning (both per-vma address space pins and + * backing storage pins at the buffer object level) result in the shrinker code + * having to skip the object. + * + * Returns: + * The number of pages of backing storage actually released. + */ unsigned long i915_gem_shrink(struct drm_i915_private *dev_priv, long target, unsigned flags) @@ -118,6 +142,20 @@ i915_gem_shrink(struct
Re: [Intel-gfx] [PATCH v2 3/7] drm/i915: Use down ei for manual Baytrail RPS calculations
On Wed, Mar 18, 2015 at 09:48:23AM +, Chris Wilson wrote: Use both up/down manual ei calcuations for symmetry and greater flexibility for reclocking, instead of faking the down interrupt based on a fixed integer number of up interrupts. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Reviewed-by: Deepak Sdeepa...@linux.intel.com Merged up to this patch, thanks. -Daniel --- drivers/gpu/drm/i915/i915_drv.h | 2 -- drivers/gpu/drm/i915/i915_irq.c | 15 ++- drivers/gpu/drm/i915/i915_reg.h | 1 - drivers/gpu/drm/i915/intel_pm.c | 5 ++--- 4 files changed, 4 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a06536cfce6d..b156bc30c9c9 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1031,8 +1031,6 @@ struct intel_gen6_power_mgmt { u8 rp0_freq;/* Non-overclocked max frequency. */ u32 cz_freq; - u32 ei_interrupt_count; - int last_adj; enum { LOW_POWER, BETWEEN, HIGH_POWER } power; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 8d8d33d068dd..6d8340d5a111 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1033,7 +1033,6 @@ void gen6_rps_reset_ei(struct drm_i915_private *dev_priv) { vlv_c0_read(dev_priv, dev_priv-rps.down_ei); dev_priv-rps.up_ei = dev_priv-rps.down_ei; - dev_priv-rps.ei_interrupt_count = 0; } static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) @@ -1041,23 +1040,13 @@ static u32 vlv_wa_c0_ei(struct drm_i915_private *dev_priv, u32 pm_iir) struct intel_rps_ei now; u32 events = 0; - if ((pm_iir GEN6_PM_RP_UP_EI_EXPIRED) == 0) + if ((pm_iir (GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED)) == 0) return 0; vlv_c0_read(dev_priv, now); if (now.cz_clock == 0) return 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) { - pm_iir |= GEN6_PM_RP_DOWN_EI_EXPIRED; - dev_priv-rps.ei_interrupt_count = 0; - } - if (pm_iir GEN6_PM_RP_DOWN_EI_EXPIRED) { if (!vlv_c0_above(dev_priv, dev_priv-rps.down_ei, now, @@ -4254,7 +4243,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv) /* Let's track the enabled rps events */ if (IS_VALLEYVIEW(dev_priv) !IS_CHERRYVIEW(dev_priv)) /* WaGsvRC0ResidencyMethod:vlv */ - dev_priv-pm_rps_events = GEN6_PM_RP_UP_EI_EXPIRED; + dev_priv-pm_rps_events = GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED; else dev_priv-pm_rps_events = GEN6_PM_RPS_EVENTS; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 2d76c566d843..5b84ee686f99 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -673,7 +673,6 @@ enum skl_disp_power_wells { #define VLV_CZ_CLOCK_TO_MILLI_SEC10 #define VLV_RP_UP_EI_THRESHOLD 90 #define VLV_RP_DOWN_EI_THRESHOLD 70 -#define VLV_INT_COUNT_FOR_DOWN_EI5 /* vlv2 north clock has */ #define CCK_FUSE_REG 0x8 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 68c9cc252d36..e18f0fd22cf2 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3922,11 +3922,10 @@ static u32 gen6_rps_pm_mask(struct drm_i915_private *dev_priv, u8 val) u32 mask = 0; if (val dev_priv-rps.min_freq_softlimit) - mask |= GEN6_PM_RP_DOWN_THRESHOLD | GEN6_PM_RP_DOWN_TIMEOUT; + mask |= GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_DOWN_THRESHOLD | GEN6_PM_RP_DOWN_TIMEOUT; if (val dev_priv-rps.max_freq_softlimit) - mask |= GEN6_PM_RP_UP_THRESHOLD; + mask |= GEN6_PM_RP_UP_EI_EXPIRED | GEN6_PM_RP_UP_THRESHOLD; - mask |= dev_priv-pm_rps_events (GEN6_PM_RP_DOWN_EI_EXPIRED | GEN6_PM_RP_UP_EI_EXPIRED); mask = dev_priv-pm_rps_events; return gen6_sanitize_rps_pm_mask(dev_priv, ~mask); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] gpio/crystalcove: Export Panel and backlight en/disable signals as GPIO
On Thu, Mar 12, 2015 at 4:06 PM, Kumar, Shobhit shobhit.ku...@intel.com wrote: On Mon, 2015-03-09 at 18:15 +0100, Linus Walleij wrote: On Fri, Mar 6, 2015 at 5:23 PM, Kumar, Shobhit shobhit.ku...@intel.com wrote: There are actually two lines for Panel Power control and Backlight enable/disable. I have already moved towards adding a new Cell device for PWM child device and a new pwm driver for the same. That will take care of backlight thingy. But for the Panel power control, I am at loss for how best to program that. Isn't it just a very simple regulator (just on/off of fixed voltage) cell? It is just behaving as an output GPIO line and for that reason I feel that rather than adding a new regulator driver, I disagree. GPIO reads general purpose input/output. This is not a GPIO, this is a special purpose thing and IMO it should be modeled directly as a regulator. Yours, Linus Walleij ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915: Handle DP_AUX_I2C_WRITE_STATUS_UPDATE
On Fri, 13 Mar 2015, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com When we get an i2c defer or short ack for i2c-over-aux write we need to switch to WRITE_STATUS_UPDATE to poll for the completion of the original request. i915 doesn't try to interpret wht request type apart from separating reads from writes, and so we should be able to treat this the same as a normal i2c write. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Jani Nikula jani.nik...@intel.com Probably needs a refresh due to context change due to my short writes patch though. --- drivers/gpu/drm/i915/intel_dp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 33d5877..aed5f26 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -956,6 +956,7 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) switch (msg-request ~DP_AUX_I2C_MOT) { case DP_AUX_NATIVE_WRITE: case DP_AUX_I2C_WRITE: + case DP_AUX_I2C_WRITE_STATUS_UPDATE: txsize = msg-size ? HEADER_SIZE + msg-size : BARE_ADDRESS_SIZE; rxsize = 1; -- 2.0.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/dp: s/I2C_STATUS/I2C_WRITE_STATUS_UPDATE/
On Fri, 13 Mar 2015, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Rename the I2C_STATUS request to I2C_WRITE_STATUS_UPDATE to match the spec. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/tegra/dpaux.c | 2 +- include/drm/drm_dp_helper.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c index d6b55e3..b1617af 100644 --- a/drivers/gpu/drm/tegra/dpaux.c +++ b/drivers/gpu/drm/tegra/dpaux.c @@ -152,7 +152,7 @@ static ssize_t tegra_dpaux_transfer(struct drm_dp_aux *aux, break; - case DP_AUX_I2C_STATUS: + case DP_AUX_I2C_WRITE_STATUS_UPDATE: if (msg-request DP_AUX_I2C_MOT) value |= DPAUX_DP_AUXCTL_CMD_MOT_RQ; else diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 523f04c..27301f5 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -46,7 +46,7 @@ #define DP_AUX_I2C_WRITE 0x0 #define DP_AUX_I2C_READ 0x1 -#define DP_AUX_I2C_STATUS0x2 +#define DP_AUX_I2C_WRITE_STATUS_UPDATE 0x2 #define DP_AUX_I2C_MOT 0x4 #define DP_AUX_NATIVE_WRITE 0x8 #define DP_AUX_NATIVE_READ 0x9 -- 2.0.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC v5 2/9] gpio/crystalcove: Add additional GPIO for Panel control
On Thu, Mar 12, 2015 at 5:31 PM, Shobhit Kumar shobhit.ku...@intel.com wrote: Export PANEL_EN/DISABLE (offset 0x52) as additional GPIO. Needed by display driver to enable the DSI panel on BYT platform where the Panel EN/Disable control is routed thorugh CRC PMIC CC: Samuel Ortiz sa...@linux.intel.com Cc: Linus Walleij linus.wall...@linaro.org Cc: Alexandre Courbot gnu...@gmail.com Cc: Thierry Reding thierry.red...@gmail.com Signed-off-by: Shobhit Kumar shobhit.ku...@intel.com NACK. Spawn a separate MFD cell and write a fixed voltage regulator driver for this. @@ -39,6 +39,7 @@ #define GPIO0P0CTLI0x33 #define GPIO1P0CTLO0x3b #define GPIO1P0CTLI0x43 +#define GPIOPANELCTL 0x52 This is even a lie, you say in the commit message that the register is indeed named PANEL_EN/DISABLE and is not a GPIO. Yours, Linus Walleij ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Log view type when printing warnings
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5984 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -1 268/268 267/268 ILK 303/303 303/303 SNB 283/283 283/283 IVB 343/343 343/343 BYT 287/287 287/287 HSW 363/363 363/363 BDW 308/308 308/308 -Detailed- Platform Testdrm-intel-nightly Series Applied *PNV igt_gem_userptr_blits_minor-sync-interruptible PASS(2) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] tests/gem_exec_lut_handle
On Wed, Mar 18, 2015 at 11:19:46AM +, Chris Wilson wrote: On Wed, Mar 18, 2015 at 11:19:32AM +0100, Daniel Vetter wrote: Reduce default number of repeats a lot. High repeat count is only useful for microbenchmarking, not that much for regression testing. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87131 This just to hide the regression that exec got a lot slower? Well 10% or so afaik. But in general I think we need to grow more smarts so that microbenchmarks don't chew too much cpu time. Eventually I'd like to integrate them somehow in our performance testing in a new igt mode or something like that ... And I haven't seen a lot of activity to track down the reloc regression itself. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915: vlv: fix save/restore of GFX_MAX_REQ_COUNT reg
Rodrigo Vivi rodrigo.v...@intel.com writes: From: Imre Deak imre.d...@intel.com Due this typo we don't save/restore the GFX_MAX_REQ_COUNT register across suspend/resume, so fix this. This was introduced in commit ddeea5b0c36f3665446518c609be91f9336ef674 Author: Imre Deak imre.d...@intel.com Date: Mon May 5 15:19:56 2014 +0300 drm/i915: vlv: add runtime PM support I noticed this only by reading the code. To my knowledge it shouldn't cause any real problems at the moment, since the power well backing this register remains on across a runtime s/r. This may change once system-wide s0ix functionality is enabled in the kernel. v2: - resend after a missing git add -u :/ Signed-off-by: Imre Deak imre.d...@intel.com Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Reviewed-by: Mika Kuoppala mika.kuopp...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 82f8be4..9cc953e 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1038,7 +1038,7 @@ static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv) s-lra_limits[i] = I915_READ(GEN7_LRA_LIMITS_BASE + i * 4); s-media_max_req_count = I915_READ(GEN7_MEDIA_MAX_REQ_COUNT); - s-gfx_max_req_count= I915_READ(GEN7_MEDIA_MAX_REQ_COUNT); + s-gfx_max_req_count= I915_READ(GEN7_GFX_MAX_REQ_COUNT); s-render_hwsp = I915_READ(RENDER_HWS_PGA_GEN7); s-ecochk = I915_READ(GAM_ECOCHK); @@ -1119,7 +1119,7 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv) I915_WRITE(GEN7_LRA_LIMITS_BASE + i * 4, s-lra_limits[i]); I915_WRITE(GEN7_MEDIA_MAX_REQ_COUNT, s-media_max_req_count); - I915_WRITE(GEN7_MEDIA_MAX_REQ_COUNT, s-gfx_max_req_count); + I915_WRITE(GEN7_GFX_MAX_REQ_COUNT, s-gfx_max_req_count); I915_WRITE(RENDER_HWS_PGA_GEN7, s-render_hwsp); I915_WRITE(GAM_ECOCHK, s-ecochk); -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Use GGTT view when (un)pinning objects to planes
On Wed, Mar 18, 2015 at 03:52:31PM +0200, Joonas Lahtinen wrote: On ti, 2015-03-17 at 15:45 +, Tvrtko Ursulin wrote: @@ -3993,7 +3994,9 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, * (e.g. libkms for the bootup splash), we have to ensure that we * always use map_and_fenceable for all scanout buffers. */ - ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE); + ret = i915_gem_object_ggtt_pin(obj, view, alignment, + view-type == I915_GGTT_VIEW_NORMAL ? + PIN_MAPPABLE : 0); I'm slightly concerned about making an assumption that other but normal views need not to be mappable (when none are defined). As discussed in IRC, this should be moved later into the series when we actually know about the other views. Just an aside: As soon as we have partial ggtt views we don't need to pin anything display related as mappable any more - we only do this to make sure we can serve any gtt mmap faults on frontbuffers, just in case. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Return current vblank value for drmWaitVBlank queries
On 03/18/2015 10:30 AM, Chris Wilson wrote: On Wed, Mar 18, 2015 at 11:53:16AM +0900, Michel Dänzer wrote: drm_vblank_count_and_time() doesn't return the correct sequence number while the vblank interrupt is disabled, does it? It returns the sequence number from the last time vblank_disable_and_save() was called (when the vblank interrupt was disabled). That's why drm_vblank_get() is needed here. Ville enlightened me as well. I thought the value was cooked so that time did not pass whilst the IRQ was disabled. Hopefully, I can impress upon the Intel folks, at least, that enabling/disabling the interrupts just to read the current hw counter is interesting to say the least and sits at the top of the profiles when benchmarking Present. -Chris drm_wait_vblank() not only gets the counter but also the corresponding vblank timestamp. Counters are recalculated in vblank_disable_and_save() for irq off, then in the vblank irq on path, and every refresh in drm_handle_vblank at vblank irq time. The timestamps can be recalculated at any time iff the driver supports high precision timestamping, which currently intel kms, radeon kms, and nouveau kms do. But for other parts, like most SoC's, afaik you only get a valid timestamp by sampling system time in the vblank irq handler, so there you'd have a problem. There are also some races around the enable/disable path which require a lot of care and exact knowledge of when each hardware fires its vblanks, updates its hardware counters etc. to get rid of them. Ville did that - successfully as far as my tests go - for Intel kms, but other drivers would be less forgiving. Our current method is to: a) Only disable vblank irqs after a default idle period of 5 seconds, so we don't get races frequent/likely enough to cause problems for clients. And we save the overhead for all the vblank irq on/off. b) On drivers which have high precision timestamping and have been carefully checked to be race free (== intel kms only atm.) we have instant disable, so things like blinking cursors don't keep vblank irq on forever. If b) causes so much overhead, maybe we could change the instant disable into a disable after a very short time, e.g., lowering the timeout from 5000 msecs to 2-3 video refresh durations ~ 50 msecs? That would still disable vblank irqs for power saving if the desktop is really idle, but avoid on/off storms for the various drm_wait_vblank's that happen when preparing a swap. -mario ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] tests/gem_exec_lut_handle
On Wed, Mar 18, 2015 at 02:50:43PM +0100, Daniel Vetter wrote: And I haven't seen a lot of activity to track down the reloc regression itself. The cause was obvious. The essence is the multiple redundant atomic operations added to execbuffer. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/7] drm/i915: Pass in plane state when (un)pinning frame buffers
On ti, 2015-03-17 at 15:45 +, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com Plane state carries the rotation information which is needed for determining the appropriate GGTT view type. This just adds the parameter with the actual usage coming in future patches. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com Reviewed-by: Joonas Lahtinen joonas.lahti...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 18 -- drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_fbdev.c | 2 +- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 16f3443..862aa46 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2252,6 +2252,7 @@ intel_fb_align_height(struct drm_device *dev, unsigned int height, int intel_pin_and_fence_fb_obj(struct drm_plane *plane, struct drm_framebuffer *fb, +const struct drm_plane_state *plane_state, Matter of taste, but I would have added the argument right before plane argument. struct intel_engine_cs *pipelined) { struct drm_device *dev = fb-dev; @@ -2339,8 +2340,11 @@ err_interruptible: return ret; } -static void intel_unpin_fb_obj(struct drm_i915_gem_object *obj) +static void intel_unpin_fb_obj(struct drm_framebuffer *fb, +const struct drm_plane_state *plane_state) { + struct drm_i915_gem_object *obj = intel_fb_obj(fb); + WARN_ON(!mutex_is_locked(obj-base.dev-struct_mutex)); i915_gem_object_unpin_fence(obj); @@ -9277,7 +9281,7 @@ static void intel_unpin_work_fn(struct work_struct *__work) enum pipe pipe = to_intel_crtc(work-crtc)-pipe; mutex_lock(dev-struct_mutex); - intel_unpin_fb_obj(intel_fb_obj(work-old_fb)); + intel_unpin_fb_obj(work-old_fb, work-crtc-primary-state); drm_gem_object_unreference(work-pending_flip_obj-base); intel_fbc_update(dev); @@ -9985,7 +9989,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, ring = dev_priv-ring[RCS]; } - ret = intel_pin_and_fence_fb_obj(crtc-primary, fb, ring); + ret = intel_pin_and_fence_fb_obj(crtc-primary, fb, + crtc-primary-state, ring); if (ret) goto cleanup_pending; @@ -10025,7 +10030,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, return 0; cleanup_unpin: - intel_unpin_fb_obj(obj); + intel_unpin_fb_obj(fb, crtc-primary-state); cleanup_pending: atomic_dec(intel_crtc-unpin_work_count); mutex_unlock(dev-struct_mutex); @@ -11981,7 +11986,7 @@ intel_prepare_plane_fb(struct drm_plane *plane, if (ret) DRM_DEBUG_KMS(failed to attach phys object\n); } else { - ret = intel_pin_and_fence_fb_obj(plane, fb, NULL); + ret = intel_pin_and_fence_fb_obj(plane, fb, new_state, NULL); } if (ret == 0) @@ -12013,7 +12018,7 @@ intel_cleanup_plane_fb(struct drm_plane *plane, if (plane-type != DRM_PLANE_TYPE_CURSOR || !INTEL_INFO(dev)-cursor_needs_physical) { mutex_lock(dev-struct_mutex); - intel_unpin_fb_obj(obj); + intel_unpin_fb_obj(fb, old_state); mutex_unlock(dev-struct_mutex); } } @@ -13906,6 +13911,7 @@ void intel_modeset_gem_init(struct drm_device *dev) if (intel_pin_and_fence_fb_obj(c-primary, c-primary-fb, +c-primary-state, NULL)) { DRM_ERROR(failed to pin boot fb on pipe %d\n, to_intel_crtc(c)-pipe); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 5254540..3721878 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -962,6 +962,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, struct intel_load_detect_pipe *old); int intel_pin_and_fence_fb_obj(struct drm_plane *plane, struct drm_framebuffer *fb, +const struct drm_plane_state *plane_state, struct intel_engine_cs *pipelined); struct drm_framebuffer * __intel_framebuffer_create(struct drm_device *dev, diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 757c0d2..4e7e7da 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -151,7 +151,7 @@ static int intelfb_alloc(struct drm_fb_helper *helper, } /* Flush
Re: [Intel-gfx] [PATCH 4/7] drm/i915: Helper function to determine GGTT view from plane state
I'd fix the below code style corrections, the functionality is ok. On ti, 2015-03-17 at 15:45 +, Tvrtko Ursulin wrote: From: Tvrtko Ursulin tvrtko.ursu...@intel.com For now only default implementation defaulting to normal view. Signed-off-by: Tvrtko Ursulin tvrtko.ursu...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 26 +++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 862aa46..fe11e99 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2249,6 +2249,16 @@ intel_fb_align_height(struct drm_device *dev, unsigned int height, fb_format_modifier)); } +static +int intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, Rather make this as it's elsewhere (static and return type on the same line): +static int +intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, + struct drm_framebuffer *fb, + const struct drm_plane_state *plane_state) +{ + *view = i915_ggtt_view_normal; + + return 0; +} + int intel_pin_and_fence_fb_obj(struct drm_plane *plane, struct drm_framebuffer *fb, @@ -2258,6 +2268,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane, struct drm_device *dev = fb-dev; struct drm_i915_private *dev_priv = dev-dev_private; struct drm_i915_gem_object *obj = intel_fb_obj(fb); + struct i915_ggtt_view view; u32 alignment; int ret; @@ -2294,6 +2305,10 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane, return -EINVAL; } + ret = intel_fill_fb_ggtt_view(view, fb, plane_state); + if (ret 0) Why not just if (ret) for consistency? + return ret; + /* Note that the w/a also requires 64 PTE of padding following the * bo. We currently fill all unused PTE with the shadow page and so * we should always have valid PTE following the scanout preventing @@ -2313,7 +2328,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane, dev_priv-mm.interruptible = false; ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined, -i915_ggtt_view_normal); +view); if (ret) goto err_interruptible; @@ -2333,7 +2348,7 @@ intel_pin_and_fence_fb_obj(struct drm_plane *plane, return 0; err_unpin: - i915_gem_object_unpin_from_display_plane(obj, i915_ggtt_view_normal); + i915_gem_object_unpin_from_display_plane(obj, view); err_interruptible: dev_priv-mm.interruptible = true; intel_runtime_pm_put(dev_priv); @@ -2344,11 +2359,16 @@ static void intel_unpin_fb_obj(struct drm_framebuffer *fb, const struct drm_plane_state *plane_state) { struct drm_i915_gem_object *obj = intel_fb_obj(fb); + struct i915_ggtt_view view; + int ret; WARN_ON(!mutex_is_locked(obj-base.dev-struct_mutex)); + ret = intel_fill_fb_ggtt_view(view, fb, plane_state); + WARN_ONCE(ret 0, Couldn't get view from plane state!); Again, just WARN_ONCE(ret, ...) as the function doesn't have need to return any positive integer result. + i915_gem_object_unpin_fence(obj); - i915_gem_object_unpin_from_display_plane(obj, i915_ggtt_view_normal); + i915_gem_object_unpin_from_display_plane(obj, view); } /* Computes the linear offset to the base tile and adjusts x, y. bytes per pixel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/dp: return number of bytes written for short aux/i2c writes
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5985 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -2 268/268 266/268 ILK 303/303 303/303 SNB 283/283 283/283 IVB 343/343 343/343 BYT 287/287 287/287 HSW 363/363 363/363 BDW 308/308 308/308 -Detailed- Platform Testdrm-intel-nightly Series Applied PNV igt_gem_userptr_blits_coherency-sync CRASH(1)PASS(3) CRASH(1)PASS(1) PNV igt_gem_tiled_pread_pwrite FAIL(1)PASS(3) FAIL(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: gen4: work around hang during hibernation
On Wed, 2015-03-18 at 12:22 +0200, Ville Syrjälä wrote: We had another bug report which showed similar problems on something as recent as SNB: https://bugzilla.kernel.org/show_bug.cgi?id=94241 So I guess we really want to make the check 'gen 7'. My IVB X1 Carbon doesn't need this quirk, so hopefully that indicates the Lenovo BIOSen became more sane for gen7+. On the other hand my ThinkPad X220 has vendor:device ids 8086:0126, which makes it a gen6 device (assuming I parsed the various preprocessor defines in include/drm/i915_pciids.h and drivers/gpu/drm/i915/i915_drv.c correctly). That laptop is now running v3.19.1 and never hit this issue. Paul Bolle ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC] drm/i195: Add flag to enable virtual mappings above 4Gb
Wa32bitGeneralStateOffset Wa32bitInstructionBaseOffset hardware workarounds require that GeneralStateOffset InstructionBaseOffset are restricted to a 32 bit address space. This is a preparatory patch prior to supporting 64bit virtual memory allocations. Allow the user space to flag that a mapping can occur beyond the 32bit limit. This allows backward compatibility and user space drivers that haven't been enhanced to support these workarounds to function. Signed-off-by: Nick Hoath nicholas.ho...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 6 ++ drivers/gpu/drm/i915/i915_gem.c | 18 +++--- include/uapi/drm/i915_drm.h | 7 ++- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3cc0196..1e6fc1d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2066,6 +2066,12 @@ struct drm_i915_gem_object { unsigned int has_dma_mapping:1; unsigned int frontbuffer_bits:INTEL_FRONTBUFFER_BITS; + + /** +* If the object should be mapped in to the bottom 4Gb +* memory space only, then this flag should not be set +*/ + unsigned int hi_mem:1; struct sg_table *pages; int pages_pin_count; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 61134ab..efa782c 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -395,7 +395,9 @@ static int i915_gem_create(struct drm_file *file, struct drm_device *dev, uint64_t size, - uint32_t *handle_p) + uint32_t *handle_p, + uint32_t flags + ) { struct drm_i915_gem_object *obj; int ret; @@ -410,6 +412,9 @@ i915_gem_create(struct drm_file *file, if (obj == NULL) return -ENOMEM; + if (flags I915_CREATE_FLAG_HI_MEM) + obj-hi_mem = 1; + ret = drm_gem_handle_create(file, obj-base, handle); /* drop reference from allocate - handle holds it now */ drm_gem_object_unreference_unlocked(obj-base); @@ -429,7 +434,8 @@ i915_gem_dumb_create(struct drm_file *file, args-pitch = ALIGN(args-width * DIV_ROUND_UP(args-bpp, 8), 64); args-size = args-pitch * args-height; return i915_gem_create(file, dev, - args-size, args-handle); + args-size, args-handle, + I915_CREATE_FLAG_HI_MEM); } /** @@ -440,9 +446,10 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data, struct drm_file *file) { struct drm_i915_gem_create *args = data; return i915_gem_create(file, dev, - args-size, args-handle); + args-size, args-handle, + args-flags); } static inline int diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 6eed16b..eb2e7d9 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -428,6 +428,8 @@ struct drm_i915_gem_init { __u64 gtt_end; }; +#define I915_CREATE_FLAG_HI_MEM0x0001 + struct drm_i915_gem_create { /** * Requested size for the object. @@ -441,7 +443,10 @@ struct drm_i915_gem_create { * Object handles are nonzero. */ __u32 handle; - __u32 pad; + /** +* Object creation flags +*/ + __u32 flags; }; struct drm_i915_gem_pread { -- 2.1.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 2/2] tests/pm_sseu: Create new test pm_sseu
On Thu, Mar 12, 2015 at 12:09:50PM +, Thomas Wood wrote: On 10 March 2015 at 21:17, jeff.mc...@intel.com wrote: From: Jeff McGee jeff.mc...@intel.com New test pm_sseu is intended for any subtest related to the slice/subslice/EU power gating feature. The sole initial subtest, 'full-enable', confirms that the slice/subslice/EU state is at full enablement when the render engine is active. Starting with Gen9 SKL, the render power gating feature can leave SSEU in a partially enabled state upon resumption of render work unless explicit action is taken. Please add a short description to the test using the IGT_TEST_DESCRIPTION macro, so that it is included in the documentation and help output. Hi Thomas. I have posted v2 patches to address this and your other comments. Can you please have a second look? Thanks -Jeff ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Turn on PIN_GLOBAL in i915_gem_object_ggtt_pin
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5986 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -2 268/268 266/268 ILK -1 303/303 302/303 SNB 283/283 283/283 IVB 343/343 343/343 BYT 287/287 287/287 HSW 363/363 363/363 BDW 308/308 308/308 -Detailed- Platform Testdrm-intel-nightly Series Applied PNV igt_gem_userptr_blits_coherency-sync CRASH(1)PASS(3) CRASH(1)PASS(1) PNV igt_gem_tiled_pread_pwrite FAIL(1)PASS(3) FAIL(1)PASS(1) *ILK igt_gem_seqno_wrap PASS(2) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915: Split the batch pool by engine
On 03/09/2015 09:55 AM, Chris Wilson wrote: I woke up one morning and found 50k objects sitting in the batch pool and every search seemed to iterate the entire list... Painting the screen in oils would provide a more fluid display. One issue with the current design is that we only check for retirements on the current ring when preparing to submit a new batch. This means that we can have thousands of active batches on another ring that we have to walk over. The simplest way to avoid that is to split the pools per ring and then our LRU execution ordering will also ensure that the inactive buffers remain at the front. Is the retire work handler not keeping up? Regularly or under some specific circumstances? v2: execlists still requires duplicate code. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_debugfs.c| 33 ++ drivers/gpu/drm/i915/i915_dma.c| 1 - drivers/gpu/drm/i915/i915_drv.h| 8 drivers/gpu/drm/i915/i915_gem.c| 2 -- drivers/gpu/drm/i915/i915_gem_batch_pool.c | 3 ++- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 +-- drivers/gpu/drm/i915/intel_lrc.c | 1 + drivers/gpu/drm/i915/intel_ringbuffer.c| 2 ++ drivers/gpu/drm/i915/intel_ringbuffer.h| 8 9 files changed, 34 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d2e6475b0466..d8ca5c89647c 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -377,13 +377,17 @@ static void print_batch_pool_stats(struct seq_file *m, { struct drm_i915_gem_object *obj; struct file_stats stats; + struct intel_engine_cs *ring; + int i; memset(stats, 0, sizeof(stats)); - list_for_each_entry(obj, - dev_priv-mm.batch_pool.cache_list, - batch_pool_list) - per_file_stats(0, obj, stats); + for_each_ring(ring, dev_priv, i) { + list_for_each_entry(obj, + ring-batch_pool.cache_list, + batch_pool_list) + per_file_stats(0, obj, stats); + } print_file_stats(m, batch pool, stats); } @@ -619,21 +623,24 @@ static int i915_gem_batch_pool_info(struct seq_file *m, void *data) struct drm_device *dev = node-minor-dev; struct drm_i915_private *dev_priv = dev-dev_private; struct drm_i915_gem_object *obj; + struct intel_engine_cs *ring; int count = 0; - int ret; + int ret, i; ret = mutex_lock_interruptible(dev-struct_mutex); if (ret) return ret; - seq_puts(m, cache:\n); - list_for_each_entry(obj, - dev_priv-mm.batch_pool.cache_list, - batch_pool_list) { - seq_puts(m,); - describe_obj(m, obj); - seq_putc(m, '\n'); - count++; + for_each_ring(ring, dev_priv, i) { + seq_printf(m, %s cache:\n, ring-name); + list_for_each_entry(obj, + ring-batch_pool.cache_list, + batch_pool_list) { + seq_puts(m,); + describe_obj(m, obj); + seq_putc(m, '\n'); + count++; + } } seq_printf(m, total: %d\n, count); diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 8e914303b831..70e050ac02b4 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1062,7 +1062,6 @@ int i915_driver_unload(struct drm_device *dev) mutex_lock(dev-struct_mutex); i915_gem_cleanup_ringbuffer(dev); - i915_gem_batch_pool_fini(dev_priv-mm.batch_pool); i915_gem_context_fini(dev); mutex_unlock(dev-struct_mutex); i915_gem_cleanup_stolen(dev); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b62e3c478221..9fe1274cfe59 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -37,7 +37,6 @@ #include intel_bios.h #include intel_ringbuffer.h #include intel_lrc.h -#include i915_gem_batch_pool.h #include i915_gem_gtt.h #include i915_gem_render_state.h #include linux/io-mapping.h @@ -1149,13 +1148,6 @@ struct i915_gem_mm { */ struct list_head unbound_list; - /* -* A pool of objects to use as shadow copies of client batch buffers -* when the command parser is enabled. Prevents the client from -* modifying the batch contents after software parsing. -*/ - struct i915_gem_batch_pool batch_pool; - /** Usable portion of the GTT for GEM */ unsigned long
Re: [Intel-gfx] [PATCH 3/6] drm/i915: Split the batch pool by engine
On Wed, Mar 18, 2015 at 04:51:58PM +, Tvrtko Ursulin wrote: On 03/09/2015 09:55 AM, Chris Wilson wrote: I woke up one morning and found 50k objects sitting in the batch pool and every search seemed to iterate the entire list... Painting the screen in oils would provide a more fluid display. One issue with the current design is that we only check for retirements on the current ring when preparing to submit a new batch. This means that we can have thousands of active batches on another ring that we have to walk over. The simplest way to avoid that is to split the pools per ring and then our LRU execution ordering will also ensure that the inactive buffers remain at the front. Is the retire work handler not keeping up? Regularly or under some specific circumstances? The retire_work handler that runs at ~1Hz? Yes, it fails to keep up. Which is why we explicitly run retire_ring before each execbuffer. diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c index 21f3356cc0ab..1287abf55b84 100644 --- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c +++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c @@ -96,8 +96,9 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, list_for_each_entry_safe(tmp, next, pool-cache_list, batch_pool_list) { +/* The batches are strictly LRU ordered */ if (tmp-active) -continue; +break; This hunk would be a better fit for 2/6, correct? No. The explanation is given by the comment + changelog. diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index fcb074bd55dc..a6d4d5502188 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1421,6 +1421,7 @@ static int logical_ring_init(struct drm_device *dev, struct intel_engine_cs *rin ring-dev = dev; INIT_LIST_HEAD(ring-active_list); INIT_LIST_HEAD(ring-request_list); +i915_gem_batch_pool_init(dev, ring-batch_pool); init_waitqueue_head(ring-irq_queue); INIT_LIST_HEAD(ring-execlist_queue); Cleanup part for _lrc ? Probably. Becomes redundant later anyway. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/6] drm/i915: Split the batch pool by engine
On 03/18/2015 05:27 PM, Chris Wilson wrote: On Wed, Mar 18, 2015 at 04:51:58PM +, Tvrtko Ursulin wrote: On 03/09/2015 09:55 AM, Chris Wilson wrote: diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c index 21f3356cc0ab..1287abf55b84 100644 --- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c +++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c @@ -96,8 +96,9 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool, list_for_each_entry_safe(tmp, next, pool-cache_list, batch_pool_list) { + /* The batches are strictly LRU ordered */ if (tmp-active) - continue; + break; This hunk would be a better fit for 2/6, correct? No. The explanation is given by the comment + changelog. I don't see this patch introducing this strict LRU ordering, rather it was there before this patch. Am I missing something? If I am not, then I see this hunk as a better fit with Tidy batch pool logic, than Split the batch pool by engine. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Keep ring-active_list and ring-requests_list consistent
If we retire requests last, we may use a later seqno and so clear the requests lists without clearing the active list, leading to confusion. Hence we should retire requests first for consistency with the early return. The order used to be important as the lifecycle for the object on the active list was determined by request-seqno. However, the requests themselves are now reference counted removing the constraint from the order of retirement. Fixes regression from commit 1b5a433a4dd967b125131da42b89b5cc0d5b1f57 Author: John Harrison john.c.harri...@intel.com Date: Mon Nov 24 18:49:42 2014 + drm/i915: Convert 'i915_seqno_passed' calls into 'i915_gem_request_completed ' and a WARNING: CPU: 0 PID: 1383 at drivers/gpu/drm/i915/i915_gem_evict.c:279 i915_gem_evict_vm+0x10c/0x140() WARN_ON(!list_empty(vm-active_list)) Identified by updating WATCH_LISTS: [drm:i915_verify_lists] *ERROR* blitter ring: active list not empty, but no requests WARNING: CPU: 0 PID: 681 at drivers/gpu/drm/i915/i915_gem.c:2751 i915_gem_retire_requests_ring+0x149/0x230() WARN_ON(i915_verify_lists(ring-dev)) Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: John Harrison john.c.harri...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem.c | 38 +- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 092f25cfb8d5..7a9589f38bbc 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2660,24 +2660,11 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring) WARN_ON(i915_verify_lists(ring-dev)); - /* Move any buffers on the active list that are no longer referenced -* by the ringbuffer to the flushing/inactive lists as appropriate, -* before we free the context associated with the requests. + /* Retire requests first as we use it above for the early return. +* If we retire requests last, we may use a later seqno and so clear +* the requests lists without clearing the active list, leading to +* confusion. */ - while (!list_empty(ring-active_list)) { - struct drm_i915_gem_object *obj; - - obj = list_first_entry(ring-active_list, - struct drm_i915_gem_object, - ring_list); - - if (!i915_gem_request_completed(obj-last_read_req, true)) - break; - - i915_gem_object_move_to_inactive(obj); - } - - while (!list_empty(ring-request_list)) { struct drm_i915_gem_request *request; @@ -2700,6 +2687,23 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring) i915_gem_free_request(request); } + /* Move any buffers on the active list that are no longer referenced +* by the ringbuffer to the flushing/inactive lists as appropriate, +* before we free the context associated with the requests. +*/ + while (!list_empty(ring-active_list)) { + struct drm_i915_gem_object *obj; + + obj = list_first_entry(ring-active_list, + struct drm_i915_gem_object, + ring_list); + + if (!i915_gem_request_completed(obj-last_read_req, true)) + break; + + i915_gem_object_move_to_inactive(obj); + } + if (unlikely(ring-trace_irq_req i915_gem_request_completed(ring-trace_irq_req, true))) { ring-irq_put(ring); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] drm/i195: Add flag to enable virtual mappings above 4Gb
On 03/18/2015 04:57 PM, Nick Hoath wrote: Wa32bitGeneralStateOffset Wa32bitInstructionBaseOffset hardware workarounds require that GeneralStateOffset InstructionBaseOffset are restricted to a 32 bit address space. This is a preparatory patch prior to supporting 64bit virtual memory allocations. Allow the user space to flag that a mapping can occur beyond the 32bit limit. This allows backward compatibility and user space drivers that haven't been enhanced to support these workarounds to function. Just on the naming.. Maybe the whole thing would better be named full mem, full range or something like that? Hi mem, especially since the name is used in the userspace API has so much historical baggage meaning it needs to be _only_ in the special high memory range. Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Return current vblank value for drmWaitVBlank queries
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 5987 -Summary- Platform Delta drm-intel-nightly Series Applied PNV -2 268/268 266/268 ILK -1 303/303 302/303 SNB 283/283 283/283 IVB 343/343 343/343 BYT 287/287 287/287 HSW 363/363 363/363 BDW 308/308 308/308 -Detailed- Platform Testdrm-intel-nightly Series Applied PNV igt_gem_userptr_blits_coherency-sync CRASH(1)PASS(3) CRASH(1)PASS(1) PNV igt_gem_tiled_pread_pwrite FAIL(1)PASS(3) FAIL(1)PASS(1) *ILK igt_drv_suspend_forcewake PASS(2) DMESG_WARN(1)PASS(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2 v2] intel: Export total subslice and EU counts
On Mon, Mar 09, 2015 at 04:13:03PM -0700, jeff.mc...@intel.com wrote: From: Jeff McGee jeff.mc...@intel.com Update kernel interface with new I915_GETPARAM ioctl entries for subslice total and EU total. Add a wrapping function for each parameter. Userspace drivers need these values when constructing GPGPU commands. This kernel query method is intended to replace the PCI ID-based tables that userspace drivers currently maintain. The kernel driver can employ fuse register reads as needed to ensure the most accurate determination of GT config attributes. This first became important with Cherryview in which the config could differ between devices with the same PCI ID. The kernel detection of these values is device-specific. Userspace drivers should continue to maintain ID-based tables for older devices which return ENODEV when using this query. v2: remove unnecessary include of stdbool.h and increment the I915_GETPARAM indices to match updated kernel patch. For: VIZ-4636 Signed-off-by: Jeff McGee jeff.mc...@intel.com Pushed to libdrm. -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915: Fix computation of last_adjustment for RPS autotuning
On Friday 06 March 2015 08:36 PM, Chris Wilson wrote: Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_irq.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9baecb79de8c..1296ce37e435 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1150,21 +1150,20 @@ static void gen6_pm_rps_work(struct work_struct *work) mutex_lock(dev_priv-rps.hw_lock); adj = dev_priv-rps.last_adj; + new_delay = dev_priv-rps.cur_freq; if (pm_iir GEN6_PM_RP_UP_THRESHOLD) { if (adj 0) adj *= 2; - else { - /* CHV needs even encode values */ - adj = IS_CHERRYVIEW(dev_priv-dev) ? 2 : 1; - } - new_delay = dev_priv-rps.cur_freq + adj; - + else /* CHV needs even encode values */ + adj = IS_CHERRYVIEW(dev_priv) ? 2 : 1; /* * For better performance, jump directly * to RPe if we're below it. */ - if (new_delay dev_priv-rps.efficient_freq) + if (new_delay dev_priv-rps.efficient_freq - adj) { new_delay = dev_priv-rps.efficient_freq; + adj = 0; + } } else if (pm_iir GEN6_PM_RP_DOWN_TIMEOUT) { if (dev_priv-rps.cur_freq dev_priv-rps.efficient_freq) new_delay = dev_priv-rps.efficient_freq; @@ -1176,24 +1175,22 @@ static void gen6_pm_rps_work(struct work_struct *work) I think we should modify adj in GEN6_PM_RP_UP_EI_EXPIRED? if not not we might request higher freq since we add adj to new_delay before request freq. Other than this. Patch looks fine Reviewed-by: Deepak S deepa...@linux.intel.com } else if (pm_iir GEN6_PM_RP_DOWN_THRESHOLD) { if (adj 0) adj *= 2; - else { - /* CHV needs even encode values */ - adj = IS_CHERRYVIEW(dev_priv-dev) ? -2 : -1; - } - new_delay = dev_priv-rps.cur_freq + adj; + else /* CHV needs even encode values */ + adj = IS_CHERRYVIEW(dev_priv) ? -2 : -1; } else { /* unknown event */ - new_delay = dev_priv-rps.cur_freq; + adj = 0; } + dev_priv-rps.last_adj = adj; + /* sysfs frequency interfaces may have snuck in while servicing the * interrupt */ + new_delay += adj; new_delay = clamp_t(int, new_delay, dev_priv-rps.min_freq_softlimit, dev_priv-rps.max_freq_softlimit); - dev_priv-rps.last_adj = new_delay - dev_priv-rps.cur_freq; - intel_set_rps(dev_priv-dev, new_delay); mutex_unlock(dev_priv-rps.hw_lock); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/19] drm/i915: Don't use encoder-new_crtc in intel_modeset_pipe_config()
-Original Message- From: Conselvan De Oliveira, Ander Sent: Friday, March 13, 2015 2:49 AM To: intel-gfx@lists.freedesktop.org Cc: Konduru, Chandra; Conselvan De Oliveira, Ander Subject: [PATCH 08/19] drm/i915: Don't use encoder-new_crtc in intel_modeset_pipe_config() Move towards atomic by using the legacy modeset's drm_atomic_state instead. v2: Move call to drm_atomic_add_affected_connectors() to intel_modeset_compute_config(). (Daniel) Signed-off-by: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6465f6d..1609628 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10435,8 +10435,11 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, { struct drm_device *dev = crtc-dev; struct intel_encoder *encoder; + struct intel_connector *connector; + struct drm_connector_state *connector_state; struct intel_crtc_state *pipe_config; int plane_bpp, ret = -EINVAL; + int i; bool retry = true; if (!check_encoder_cloning(to_intel_crtc(crtc))) { @@ -10510,11 +10513,17 @@ encoder_retry: * adjust it according to limitations or connector properties, and also * a chance to reject the mode entirely. */ - for_each_intel_encoder(dev, encoder) { + for (i = 0; i state-num_connector; i++) { + connector = to_intel_connector(state-connectors[i]); + if (!connector) + continue; - if (encoder-new_crtc-base != crtc) + connector_state = state-connector_states[i]; + if (connector_state-crtc != crtc) continue; + encoder = to_intel_encoder(connector_state-best_encoder); + if (!(encoder-compute_config(encoder, pipe_config))) { DRM_DEBUG_KMS(Encoder config failure\n); goto fail; @@ -11238,6 +11247,10 @@ intel_modeset_compute_config(struct drm_crtc *crtc, struct intel_crtc *intel_crtc; int ret = 0; + ret = drm_atomic_add_affected_connectors(state, crtc); If the current transaction is started by DRM core, above operation will be added by core, right? And when we move to full atomic_modeset, then above needs to be deleted? + if (ret) + return ERR_PTR(ret); + intel_modeset_affected_pipes(crtc, modeset_pipes, prepare_pipes, disable_pipes); -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] Add Dmesg Triage Feature: further triage i-g-t kmsg log to reduce result noise resulted from piglit dmesg defect
tests/igt.py: add igt env to enable or disable dmesg triage framework/test/base.py: trigger dmesg triage depending on dmesg log occurrence framework/dmesg.py: employ dmesg triage simply for Linux dmesg dmesg_triage/*: deal with kmsg log with pre-defined dmesg oops pattern In general, if dmesg triage is enabled and there is new dmesg along with i-g-t testcases running, the new dmesg will be captured and worked out a tag and head to rollback the detail when necessary, in addition, on the basis of different dmesg tag or head, the final result noise of a testcase can be reduced with defined rules or strategy. Signed-off-by: ethan gao ethan@intel.com --- dmesg_triage/debug.sh | 67 ++ dmesg_triage/dmesg.rb | 117 dmesg_triage/kmsg_triage | 182 ++ dmesg_triage/libdmesg.sh | 156 dmesg_triage/oops-context-pattern | 37 dmesg_triage/oops-pattern | 59 framework/dmesg.py| 60 + framework/test/base.py| 4 + tests/igt.py | 7 ++ 9 files changed, 689 insertions(+) create mode 100644 dmesg_triage/debug.sh create mode 100644 dmesg_triage/dmesg.rb create mode 100755 dmesg_triage/kmsg_triage create mode 100644 dmesg_triage/libdmesg.sh create mode 100644 dmesg_triage/oops-context-pattern create mode 100644 dmesg_triage/oops-pattern diff --git a/dmesg_triage/debug.sh b/dmesg_triage/debug.sh new file mode 100644 index 000..ec8280e --- /dev/null +++ b/dmesg_triage/debug.sh @@ -0,0 +1,67 @@ +#!/bin/bash + +# Copyright (c) 2015 Intel Corporation + +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the Software), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: + +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. + +# THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +[[ $BASHPID ]] || echo === Run me with bash === + +dump_shell_vars() +{ + ( + set -o posix + local branch_commit_files=() + local sparse_lines=() + local remote_url=() + set 2 + ) +} + +dump_call_stack() +{ + local stack_depth=${#FUNCNAME[@]} + local i + for ((i = 0; i $((stack_depth-1)); i++)); do + echo ${BASH_SOURCE[i+1]}:${BASH_LINENO[i]}: ${FUNCNAME[i+1]} 2 + done +} + +# nr_stack_dumps=0 +dump_stack() +{ + # echo Stack dump: $BASH_COMMAND + dump_call_stack + echo + dump_shell_vars +} + +notice() +{ + local time_str=$(date +'%F %H:%M:%S') + echo -e ${color[MAGENTA]}${time_str}$*$reset_color +} + +die() +{ + notice $* + + dump_stack + email $* + exit +} diff --git a/dmesg_triage/dmesg.rb b/dmesg_triage/dmesg.rb new file mode 100644 index 000..d7aecae --- /dev/null +++ b/dmesg_triage/dmesg.rb @@ -0,0 +1,117 @@ +#!/usr/bin/ruby + +# Copyright (c) 2015 Intel Corporation + +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the Software), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: + +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. + +# THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +# [Description]: Find out all crash dmesg clauses with
Re: [Intel-gfx] [PATCH v4] drm/i915: Allocate a drm_atomic_state for the legacy modeset code
-Original Message- From: Conselvan De Oliveira, Ander Sent: Wednesday, March 18, 2015 12:57 AM To: intel-gfx@lists.freedesktop.org Cc: Konduru, Chandra; Conselvan De Oliveira, Ander Subject: [PATCH v4] drm/i915: Allocate a drm_atomic_state for the legacy modeset code For the atomic conversion, the mode set paths need to be changed to rely on an atomic state instead of using the staged config. By using an atomic state for the legacy code, we will be able to convert the code base in small chunks. v2: Squash patch that adds stat argument to intel_set_mode(). (Ander) Make every caller of intel_set_mode() allocate state. (Daniel) Call drm_atomic_state_clear() in set config's error path. (Daniel) v3: Copy staged config to atomic state in force restore path. (Ander) v4: Don't update -new_config for disabled pipes in __intel_set_mode(), since it is expected to be NULL in that case. (Ander) Can you clarify why it is expected to be NULL? Signed-off-by: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 200 +- - 1 file changed, 165 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8458bf5..ce35647 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -83,7 +83,8 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc, struct intel_crtc_state *pipe_config); static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode, - int x, int y, struct drm_framebuffer *old_fb); + int x, int y, struct drm_framebuffer *old_fb, + struct drm_atomic_state *state); static int intel_framebuffer_init(struct drm_device *dev, struct intel_framebuffer *ifb, struct drm_mode_fb_cmd2 *mode_cmd, @@ -8802,6 +8803,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector, struct drm_device *dev = encoder-dev; struct drm_framebuffer *fb; struct drm_mode_config *config = dev-mode_config; + struct drm_atomic_state *state = NULL; int ret, i = -1; DRM_DEBUG_KMS([CONNECTOR:%d:%s], [ENCODER:%d:%s]\n, @@ -8883,6 +8885,12 @@ retry: old-load_detect_temp = true; old-release_fb = NULL; + state = drm_atomic_state_alloc(dev); + if (!state) + return false; + + state-acquire_ctx = ctx; + if (!mode) mode = load_detect_mode; @@ -8905,7 +8913,7 @@ retry: goto fail; } - if (intel_set_mode(crtc, mode, 0, 0, fb)) { + if (intel_set_mode(crtc, mode, 0, 0, fb, state)) { DRM_DEBUG_KMS(failed to set mode on load-detect pipe\n); if (old-release_fb) old-release_fb-funcs-destroy(old-release_fb); @@ -8924,6 +8932,11 @@ retry: else intel_crtc-new_config = NULL; fail_unlock: + if (state) { + drm_atomic_state_free(state); + state = NULL; + } + if (ret == -EDEADLK) { drm_modeset_backoff(ctx); goto retry; @@ -8936,22 +8949,34 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, struct intel_load_detect_pipe *old, struct drm_modeset_acquire_ctx *ctx) { + struct drm_device *dev = connector-dev; struct intel_encoder *intel_encoder = intel_attached_encoder(connector); struct drm_encoder *encoder = intel_encoder-base; struct drm_crtc *crtc = encoder-crtc; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct drm_atomic_state *state; DRM_DEBUG_KMS([CONNECTOR:%d:%s], [ENCODER:%d:%s]\n, connector-base.id, connector-name, encoder-base.id, encoder-name); if (old-load_detect_temp) { + state = drm_atomic_state_alloc(dev); + if (!state) { + DRM_DEBUG_KMS(can't release load detect pipe\n); + return; + } + + state-acquire_ctx = ctx; + to_intel_connector(connector)-new_encoder = NULL; intel_encoder-new_crtc = NULL; intel_crtc-new_enabled = false; intel_crtc-new_config = NULL; - intel_set_mode(crtc, NULL, 0, 0, NULL); + intel_set_mode(crtc, NULL, 0, 0, NULL, state); + + drm_atomic_state_free(state); if (old-release_fb) { drm_framebuffer_unregister_private(old-release_fb); @@ -10345,10 +10370,22 @@ static bool check_digital_port_conflicts(struct drm_device *dev) return true;
Re: [Intel-gfx] [PATCH v2 00/19] Remove depencies on staged config for atomic transition
-Original Message- From: Conselvan De Oliveira, Ander Sent: Friday, March 13, 2015 2:49 AM To: intel-gfx@lists.freedesktop.org Cc: Konduru, Chandra; Conselvan De Oliveira, Ander Subject: [PATCH v2 00/19] Remove depencies on staged config for atomic transition Here's v2 with most of the comments from Daniel addressed. I didn't change the zeroing of the crtc_state to the duplicate state function, since it causes problems when we add the state of a crtc that isn't going through a modeset. Specifically, this would cause the code that decides whether pipes B and C can be enabled at the same time to fail, since the number of fdi lanes would be zero. The other concerns I raised with v1 are also addressed. I tested this on IVYBRIDGE using pipes B C, and the load detect code path using Daniel's patch that adds the i915.load_detect_test parameter. Had a chat with Ander to get an overview of his patches. It is helping to review the patches. There are still couple questions, so will ask for clarifications as I go through the changes. Also got clarified that this patch series is moving in direction for atomic_crtc but more work needs to be done to separate out check path, swap states then commit path. And hook these check and commit paths into intel_atomic_check/commit functions. Also any resources involved in doing internally induced set_mode, update_plane needs to be added to incoming nuclear/atomic transaction. I think this is one of the major todo. In that process need find way to avoid nested atomic transactions. Also complications can arise if the transaction already has crtc state to do a mode change, and requires another set mode to do load detect. In these type of scenarios, suspend calling nuclear transaction and start a freshone? Or do both at same time. It is not clear how this can be done. Probably Daniel has some ideas to overcome these. Current nightly has .update_plane pointing to helper upate instead of atomic_helper_update to avoid nested atomic. Thanks, Ander Ander Conselvan de Oliveira (19): drm/i915: Add intel_atomic_get_crtc_state() helper function drm/i915: Pass acquire ctx also to intel_release_load_detect_pipe() drm/i915: Allocate a drm_atomic_state for the legacy modeset code drm/i915: Allocate a crtc_state also when the crtc is being disabled drm/i915: Update dummy connector atomic state with current config drm/i915: Implement connector state duplication drm/i915: Copy the staged connector config to the legacy atomic state drm/i915: Don't use encoder-new_crtc in intel_modeset_pipe_config() drm/i915: Don't use encoder-new_crtc in compute_baseline_pipe_bpp() drm/i915: Don't depend on encoder-new_crtc in intel_dp_compute_config() drm/i915: Don't depend on encoder-new_crtc in intel_hdmi_compute_config drm/i915: Use atomic state in intel_ddi_crtc_get_new_encoder() drm/i915: Don't use staged config in intel_dp_mst_compute_config() drm/i915: Don't use encoder-new_crtc in intel_lvds_compute_config() drm/i915: Pass an atomic state to modeset_global_resources() functions drm/i915: Check lane sharing between pipes B C using atomic state drm/i915: Convert intel_pipe_will_have_type() to using atomic state drm/i915: Don't look at staged config crtc when changing DRRS state drm/i915: Remove usage of encoder-new_crtc from clock computations drivers/gpu/drm/i915/i915_drv.h | 4 +- drivers/gpu/drm/i915/intel_crt.c | 3 +- drivers/gpu/drm/i915/intel_ddi.c | 24 +- drivers/gpu/drm/i915/intel_display.c | 544 ++- drivers/gpu/drm/i915/intel_dp.c | 5 +- drivers/gpu/drm/i915/intel_dp_mst.c | 18 +- drivers/gpu/drm/i915/intel_drv.h | 16 +- drivers/gpu/drm/i915/intel_dsi.c | 1 + drivers/gpu/drm/i915/intel_dvo.c | 1 + drivers/gpu/drm/i915/intel_hdmi.c| 22 +- drivers/gpu/drm/i915/intel_lvds.c| 3 +- drivers/gpu/drm/i915/intel_sdvo.c| 1 + drivers/gpu/drm/i915/intel_tv.c | 3 +- 13 files changed, 484 insertions(+), 161 deletions(-) -- 2.1.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [Beignet] Preventing zero GPU virtual address allocation
Yeah, MAP_FIXED sounds a bit more ambitious and though I think it would work for OCL 2.0 pointer sharing, it's a little different than we were planning. To summarize, we have three possible approaches, each with its own problems: 1) simple patch to avoid binding at address 0 in PPGTT: does impact the ABI (though generally not in a harmful way), and may not be possible with aliasing PPGTT with e.g. framebuffers bound at offset 0 2) exposing PIN_BIAS to userspace Would allow userspace to avoid pinning any buffers at offset 0 at execbuf time, but still has the problem with previously bound buffers and aliasing PPGTT 3) MAP_FIXED interface Flexible approach allowing userspace to manage its own virtual memory, but still has the same issues with aliasing PPGTT, and with shared contexts, which would have to negotiate between libraries how to handle the zero page For (1) and (2) the kernel pieces are really already in place, the main thing we need is a new flag to userspace to indicate behavior. I'd prefer (1) with a context creation flag to indicate don't bind at 0. Execbuf would try to honor this, and userspace could check if any buffers ended up at 0 in the aliasing PPGTT case by checking the resulting offsets following the call. I expect in most cases this would be fine. It should be pretty easy to extend Ruiling's patch to use a context flag to determine the behavior; is that something you can do? Any objections to this approach? I am ok with adding a context flag to indicate don't bind at 0. Any objections from others? The patch is not from me, it is from David. I am not familiar with KMD. David, could you help on this patch? It does mean that shared contexts need to be handled specially, or won't get the 0 page protection, but I think Mesa wants this behavior too, and libva probably wouldn't mind, so you could just require new versions of those that set this flag when telling people what's supported for proper NULL pointer handling. Any objections to that approach? Thanks, Jesse ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Async eDP init
This updates my old patch for this, but w/o fixing the locking issue Ville mentioned. In looking at it, it seems like the sync point should be at a higher level, maybe at the level of the atomic mode setting async serialization points? Another possibility would be to make it a lazy init type function, sprinkled about but only running once when we first need it. Any thoughts from anyone? I don't think I can just do a lock drop here, since other threads may jump in and mess with underlying state. That shouldn't affect the eDP state we fill out, but may affect the state the caller depended on in the first place... Thanks, Jesse ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC] drm/i915: don't wait_for_vblank if the CRTC is disabled
From: Paulo Zanoni paulo.r.zan...@intel.com Otherwise we'll get a WARN from drm_wait_one_vblank() saying that vblanks are not available (since they were already disabled in crtc_disable()). This is certainly a regresison, but QA couldn't bisect it due to other regressions breaking the bisect. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89550 Testcase: igt/pm_rpm/legacy-planes Testcase: igt/pm_rpm/legacy-planes-dpms Testcase: igt/pm_rpm/universal-planes Testcase: igt/pm_rpm/universal-planes-dpms Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Matt Roper matthew.d.ro...@intel.com Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) I'm not really sure if this is the best way to fix the regression. Ville and/or Matt should provide some comments here. diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f1c0295..f2f7e81 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12193,7 +12193,7 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc) intel_runtime_pm_put(dev_priv); - if (intel_crtc-atomic.wait_vblank) + if (intel_crtc-active intel_crtc-atomic.wait_vblank) intel_wait_for_vblank(dev, intel_crtc-pipe); intel_frontbuffer_flip(dev, intel_crtc-atomic.fb_bits); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] intel: Export total subslice and EU counts
On Mon, Mar 02, 2015 at 03:39:27PM -0800, jeff.mc...@intel.com wrote: From: Jeff McGee jeff.mc...@intel.com 2 small details, but otherwise: Reviewed-by: Damien Lespiau damien.lesp...@intel.com Update kernel interface with new I915_GETPARAM ioctl entries for subslice total and EU total. Add a wrapping function for each parameter. Userspace drivers need these values when constructing GPGPU commands. This kernel query method is intended to replace the PCI ID-based tables that userspace drivers currently maintain. The kernel driver can employ fuse register reads as needed to ensure the most accurate determination of GT config attributes. This first became important with Cherryview in which the config could differ between devices with the same PCI ID. The kernel detection of these values is device-specific. Userspace drivers should continue to maintain ID-based tables for older devices which return ENODEV when using this query. This should probably part of some comment near the API entry point. For: VIZ-4636 Signed-off-by: Jeff McGee jeff.mc...@intel.com --- include/drm/i915_drm.h | 2 ++ intel/intel_bufmgr.h | 4 intel/intel_bufmgr_gem.c | 31 +++ 3 files changed, 37 insertions(+) diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h index 15dd01d..e34f5b2 100644 --- a/include/drm/i915_drm.h +++ b/include/drm/i915_drm.h @@ -340,6 +340,8 @@ typedef struct drm_i915_irq_wait { #define I915_PARAM_HAS_EXEC_HANDLE_LUT 26 #define I915_PARAM_HAS_WT 27 #define I915_PARAM_CMD_PARSER_VERSION 28 +#define I915_PARAM_SUBSLICE_TOTAL 32 +#define I915_PARAM_EU_TOTAL 33 typedef struct drm_i915_getparam { int param; diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h index be83a56..4b2472e 100644 --- a/intel/intel_bufmgr.h +++ b/intel/intel_bufmgr.h @@ -37,6 +37,7 @@ #include stdio.h #include stdint.h #include stdio.h +#include stdbool.h But you don't seem to use bool or _Bool in the rest of the patch? struct drm_clip_rect; @@ -264,6 +265,9 @@ int drm_intel_get_reset_stats(drm_intel_context *ctx, uint32_t *active, uint32_t *pending); +int drm_intel_get_subslice_total(int fd, unsigned int *subslice_total); +int drm_intel_get_eu_total(int fd, unsigned int *eu_total); + /** @{ Compatibility defines to keep old code building despite the symbol rename * from dri_* to drm_intel_* */ diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c index 78875fd..2d77f32 100644 --- a/intel/intel_bufmgr_gem.c +++ b/intel/intel_bufmgr_gem.c @@ -3292,6 +3292,37 @@ drm_intel_reg_read(drm_intel_bufmgr *bufmgr, return ret; } +drm_public int +drm_intel_get_subslice_total(int fd, unsigned int *subslice_total) +{ + drm_i915_getparam_t gp; + int ret; + + memclear(gp); + gp.value = (int*)subslice_total; + gp.param = I915_PARAM_SUBSLICE_TOTAL; + ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, gp); + if (ret) + return -errno; + + return 0; +} + +drm_public int +drm_intel_get_eu_total(int fd, unsigned int *eu_total) +{ + drm_i915_getparam_t gp; + int ret; + + memclear(gp); + gp.value = (int*)eu_total; + gp.param = I915_PARAM_EU_TOTAL; + ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, gp); + if (ret) + return -errno; + + return 0; +} /** * Annotate the given bo for use in aub dumping. -- 2.3.0 ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915/dp: move edp init to work queue
This helps speed up driver init time, and puts off the eDP stuff until we actually need it. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org --- drivers/gpu/drm/i915/intel_dp.c | 103 ++- drivers/gpu/drm/i915/intel_drv.h | 3 ++ 2 files changed, 73 insertions(+), 33 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 5256c06..2abd339 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3768,6 +3768,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) DRM_DEBUG_KMS(DPCD: %*ph\n, (int) sizeof(intel_dp-dpcd), intel_dp-dpcd); + if (intel_dp-dpcd_valid) + return true; + if (intel_dp-dpcd[DP_DPCD_REV] == 0) return false; /* DPCD not present */ @@ -4012,6 +4015,25 @@ go_again: return -EINVAL; } +static void intel_flush_edp_cache_work(struct intel_dp *intel_dp) +{ + struct drm_device *dev = intel_dp-attached_connector-base.dev; + + WARN_ON(!mutex_is_locked(dev-mode_config.mutex)); + + if (!is_edp(intel_dp)) + return; + + /* +* FIXME: we need to synchronize this at a higher level, like the +* first mode set or other display I/O activity. Maybe re-use +* async mode setting entry points? +*/ + mutex_unlock(dev-mode_config.mutex); + flush_work(intel_dp-edp_cache_work); + mutex_lock(dev-mode_config.mutex); +} + /* * According to DP spec * 5.1.2: @@ -4044,6 +4066,8 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) return; } + intel_flush_edp_cache_work(intel_dp); + /* Now read the DPCD to see if it's actually running */ if (!intel_dp_get_dpcd(intel_dp)) { return; @@ -4079,6 +4103,8 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) uint8_t *dpcd = intel_dp-dpcd; uint8_t type; + intel_flush_edp_cache_work(intel_dp); + if (!intel_dp_get_dpcd(intel_dp)) return connector_status_disconnected; @@ -4215,13 +4241,23 @@ g4x_dp_detect(struct intel_dp *intel_dp) return intel_dp_detect_dpcd(intel_dp); } +static bool intel_connector_has_edid(struct intel_connector *intel_connector) +{ + struct intel_dp *intel_dp = intel_attached_dp(intel_connector-base); + struct drm_device *dev = intel_dp_to_dev(intel_dp); + + intel_flush_edp_cache_work(intel_dp); + + return intel_connector-edid != NULL; +} + static struct edid * intel_dp_get_edid(struct intel_dp *intel_dp) { struct intel_connector *intel_connector = intel_dp-attached_connector; /* use cached edid if we have one */ - if (intel_connector-edid) { + if (intel_connector_has_edid(intel_connector)) { /* invalid edid */ if (IS_ERR(intel_connector-edid)) return NULL; @@ -4516,6 +4552,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) intel_dp_mst_encoder_cleanup(intel_dig_port); if (is_edp(intel_dp)) { cancel_delayed_work_sync(intel_dp-panel_vdd_work); + cancel_work_sync(intel_dp-edp_cache_work); /* * vdd might still be enabled do to the delayed vdd off. * Make sure vdd is actually turned off here. @@ -5316,9 +5353,11 @@ intel_dp_drrs_init(struct intel_connector *intel_connector, return downclock_mode; } -static bool intel_edp_init_connector(struct intel_dp *intel_dp, -struct intel_connector *intel_connector) +static void intel_edp_cache_work(struct work_struct *work) { + struct intel_dp *intel_dp = container_of(work, struct intel_dp, +edp_cache_work); + struct intel_connector *intel_connector = intel_dp-attached_connector; struct drm_connector *connector = intel_connector-base; struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct intel_encoder *intel_encoder = intel_dig_port-base; @@ -5329,12 +5368,10 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, bool has_dpcd; struct drm_display_mode *scan; struct edid *edid; - enum pipe pipe = INVALID_PIPE; dev_priv-drrs.type = DRRS_NOT_SUPPORTED; - if (!is_edp(intel_dp)) - return true; + mutex_lock(dev-mode_config.mutex); pps_lock(intel_dp); intel_edp_panel_vdd_sanitize(intel_dp); @@ -5348,10 +5385,13 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, dev_priv-no_aux_handshake = intel_dp-dpcd[DP_MAX_DOWNSPREAD] DP_NO_AUX_HANDSHAKE_LINK_TRAINING; + intel_dp-dpcd_valid = true; } else { - /* if this fails, presume the device
Re: [Intel-gfx] [RFC] drm/i915: don't wait_for_vblank if the CRTC is disabled
On Wed, Mar 18, 2015 at 04:15:07PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Otherwise we'll get a WARN from drm_wait_one_vblank() saying that vblanks are not available (since they were already disabled in crtc_disable()). This is certainly a regresison, but QA couldn't bisect it due to other regressions breaking the bisect. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89550 Testcase: igt/pm_rpm/legacy-planes Testcase: igt/pm_rpm/legacy-planes-dpms Testcase: igt/pm_rpm/universal-planes Testcase: igt/pm_rpm/universal-planes-dpms Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Matt Roper matthew.d.ro...@intel.com Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) I'm not really sure if this is the best way to fix the regression. Ville and/or Matt should provide some comments here. This will definitely fix the problem (we shouldn't be waiting for vblank with a disabled CRTC!), but I think the true bug in our code is that our sprite commit function is setting some bits that should have been set back in the 'check' phase under the 'if (intel_crtc-active)' branch. I'll supply a patch shortly that I think should fix how we got into this situation in the first place. The whole 'wait_for_vblank' flag is something we should be able to get rid of completely once I finish the atomic watermark work. Matt diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f1c0295..f2f7e81 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12193,7 +12193,7 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc) intel_runtime_pm_put(dev_priv); - if (intel_crtc-atomic.wait_vblank) + if (intel_crtc-active intel_crtc-atomic.wait_vblank) intel_wait_for_vblank(dev, intel_crtc-pipe); intel_frontbuffer_flip(dev, intel_crtc-atomic.fb_bits); -- 2.1.4 -- Matt Roper Graphics Software Engineer IoTG Platform Enabling Development Intel Corporation (916) 356-2795 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Move vblank wait determination to 'check' phase
Determining whether we'll need to wait for vblanks is something we should determine during the atomic 'check' phase, not the 'commit' phase. Note that we only set these bits in the branch of 'check' where intel_crtc-active is true so that we don't try to wait on a disabled CRTC. The whole 'wait for vblank after update' flag should go away in the future, once we start handling watermarks in a proper atomic manner. Cc: Paulo Zanoni paulo.r.zan...@intel.com Root-cause-analysis-by: Paulo Zanoni paulo.r.zan...@intel.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89550 Testcase: igt/pm_rpm/legacy-planes Testcase: igt/pm_rpm/legacy-planes-dpms Testcase: igt/pm_rpm/universal-planes Testcase: igt/pm_rpm/universal-planes-dpms Signed-off-by: Matt Roper matthew.d.ro...@intel.com --- Paulo, can you test this patch and see if it solves the problem? The only platform I have access to (IVB) doesn't have runtime power management, so I can't actually run the relevant testcases myself. drivers/gpu/drm/i915/intel_sprite.c | 24 ++-- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index a828736..fad1d9f 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -747,13 +747,6 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) I915_WRITE(SPRSURF(pipe), 0); intel_flush_primary_plane(dev_priv, intel_crtc-plane); - - /* -* Avoid underruns when disabling the sprite. -* FIXME remove once watermark updates are done properly. -*/ - intel_crtc-atomic.wait_vblank = true; - intel_crtc-atomic.update_sprite_watermarks |= (1 drm_plane_index(plane)); } static int @@ -937,13 +930,6 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc) I915_WRITE(DVSSURF(pipe), 0); intel_flush_primary_plane(dev_priv, intel_crtc-plane); - - /* -* Avoid underruns when disabling the sprite. -* FIXME remove once watermark updates are done properly. -*/ - intel_crtc-atomic.wait_vblank = true; - intel_crtc-atomic.update_sprite_watermarks |= (1 drm_plane_index(plane)); } /** @@ -1262,6 +1248,16 @@ finish: plane-state-fb-modifier[0] != state-base.fb-modifier[0]) intel_crtc-atomic.update_wm = true; + + if (!state-visible) { + /* +* Avoid underruns when disabling the sprite. +* FIXME remove once watermark updates are done properly. +*/ + intel_crtc-atomic.wait_vblank = true; + intel_crtc-atomic.update_sprite_watermarks |= + (1 drm_plane_index(plane)); + } } return 0; -- 1.8.5.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fallback to using unmappable memory for scanout
The existing ABI says that scanouts are pinned into the mappable region so that legacy clients (e.g. old Xorg or plymouthd) can write directly into the scanout through a GTT mapping. However if the surface does not fit into the mappable region, we are better off just trying to fit it anywhere and hoping for the best. (Any userspace that is cappable of using ginormous scanouts is also likely not to rely on pure GTT updates.) In the future, there may even be a kernel mediated method for the legacy clients. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Satyanantha, Rama Gopal M rama.gopal.m.satyanan...@intel.com Cc: Deepak S deepa...@linux.intel.com Cc: Damien Lespiau damien.lesp...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 9e498e0bbf22..9a1de848e450 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4034,10 +4034,15 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, /* As the user may map the buffer once pinned in the display plane * (e.g. libkms for the bootup splash), we have to ensure that we -* always use map_and_fenceable for all scanout buffers. +* always use map_and_fenceable for all scanout buffers. However, +* it may simply be too big to fit into mappable, in which case +* put it anyway and hope that userspace can cope (but always first +* try to preserve the existing ABI). */ ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE); if (ret) + ret = i915_gem_obj_ggtt_pin(obj, alignment, 0); + if (ret) goto err_unpin_display; i915_gem_object_flush_cpu_write_domain(obj); -- 2.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/21] drm/i915: Preserve scaler state when clearing crtc_state
On Tue, 2015-03-17 at 18:11 +, Konduru, Chandra wrote: -Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, March 17, 2015 7:17 AM To: Konduru, Chandra Cc: intel-gfx@lists.freedesktop.org; Conselvan De Oliveira, Ander; Vetter, Daniel Subject: Re: [Intel-gfx] [PATCH 13/21] drm/i915: Preserve scaler state when clearing crtc_state On Sat, Mar 14, 2015 at 10:55:38PM -0700, Chandra Konduru wrote: crtc_state is cleared during mode set which wipes out complete scaler state too. This is causing issues. To fix, ensure scaler state is preserved because it contains not only crtc scaler usage, but also planes using scalers on this crtc. Signed-off-by: Chandra Konduru chandra.kond...@intel.com I guess this is going to conflict with Ander's crtc state series. But since you're signed up to review that I guess you will know what needs to be changed here. Hopefully Ander's series will just take care of this. Planes may have changed its scaling needs and updated scaler_users is in crtc_state. Wiping out crtc state will wipeout staged scaler_state and cause incorrect behavior. So preserving computed (or staged) scaler state into crtc state. Ander, Can you pls check for any issue here? As is your patch shouldn't cause any trouble. The problem we have is that the legacy modeset path assumes the new pipe_config is kzalloc'd. I need to inspect all that code and check what really depends on the state being zeroed, but I haven't got to it yet. Once that's done, we could just duplicate the whole crtc state and be done with it. Ander -Daniel --- drivers/gpu/drm/i915/intel_display.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e07c24e..9c3c6b1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10617,11 +10617,14 @@ static void clear_intel_crtc_state(struct intel_crtc_state *crtc_state) { struct drm_crtc_state tmp_state; + struct intel_crtc_scaler_state scaler_state; - /* Clear only the intel specific part of the crtc state */ + /* Clear only the intel specific part of the crtc state excluding +scalers */ tmp_state = crtc_state-base; + scaler_state = crtc_state-scaler_state; memset(crtc_state, 0, sizeof *crtc_state); crtc_state-base = tmp_state; + crtc_state-scaler_state = scaler_state; } static int -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx