Re: [Intel-gfx] [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.
Hi Daniel,
On Mon, 2014-08-04 at 10:07 +0200, Daniel Vetter wrote:
On Fri, Aug 01, 2014 at 12:34:56PM +0530, sagar.a.kam...@intel.com wrote:
From: Sagar Kamble sagar.a.kam...@intel.com
Sequence to get gfx clocks on/off, allow/disallow wake and save/restore of
gunit registers need to be followed in
PM suspend and resume path similar to runtime suspend and resume.
v2:
1. Keeping GT access, wake, gunit save/restore related helpers static.
2. Moved GT access check, Wake Control, Gunit state save to end of
i915_drm_freeze.
3. Reusing the sequence in runtime_suspend/resume path at macro level.
Cc: Imre Deak imre.deak at intel.com
Cc: Paulo Zanoni paulo.r.zanoni at intel.com
Cc: Daniel Vetter daniel.vetter at ffwll.ch
Cc: Jani Nikula jani.nikula at linux.intel.com
Cc: Goel, Akash akash.g...@intel.com
Change-Id: I15cfdeeec9c976d9839bb281f809664f4a0c78a2
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 39
+--
drivers/gpu/drm/i915/i915_drv.h | 1 +
2 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c
b/drivers/gpu/drm/i915/i915_drv.c
index 6c4b25c..385dc74 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -490,11 +490,16 @@ bool i915_semaphore_is_enabled(struct drm_device *dev)
return true;
}
+static int vlv_runtime_suspend(struct drm_i915_private *dev_priv);
+static int vlv_runtime_resume(struct drm_i915_private *dev_priv,
+ bool resume_from_s0ix);
+
static int i915_drm_freeze(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_crtc *crtc;
pci_power_t opregion_target_state;
+ int ret = 0;
/* ignore lid events during suspend */
mutex_lock(dev_priv-modeset_restore_lock);
@@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev)
intel_display_set_init_power(dev_priv, false);
- return 0;
+ /* Save Gunit State and clear wake - Need to make sure
+* changes in vlv_runtime_suspend path don't impact this path */
+ if (IS_VALLEYVIEW(dev))
+ ret = vlv_runtime_suspend(dev_priv);
Maybe I wasn't clear, but I absolutely don't want any IS_VLV additions to
core resume/thaw code. This should be shovelled into the runtime pm
handling code, which should be reused in the suspend/resume code.
This piece of code does not fit into any of the power well get/put path.
Its specific sequence that need to be followed in VLV when Gunit gets
power gated. So we have to keep this IS_VLV related functionality in
both runtime and pm suspend/resume.
+
+ return ret;
}
int i915_suspend(struct drm_device *dev, pm_message_t state)
@@ -610,6 +620,12 @@ void intel_console_resume(struct work_struct *work)
static int i915_drm_thaw_early(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
+ int ret = 0;
+
+ /* Restore Gunit State and allow wake - Need to make sure
+* changes in vlv_runtime_resume path don't impact this path */
+ if (IS_VALLEYVIEW(dev))
+ ret = vlv_runtime_resume(dev_priv, true);
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
hsw_disable_pc8(dev_priv);
@@ -618,7 +634,7 @@ static int i915_drm_thaw_early(struct drm_device *dev)
intel_uncore_sanitize(dev);
intel_power_domains_init_hw(dev_priv);
- return 0;
+ return ret;
}
static int __i915_drm_thaw(struct drm_device *dev, bool
restore_gtt_mappings)
@@ -1098,6 +1114,7 @@ static void vlv_save_gunit_s0ix_state(struct
drm_i915_private *dev_priv)
s-gu_ctl0 = I915_READ(VLV_GU_CTL0);
s-gu_ctl1 = I915_READ(VLV_GU_CTL1);
s-clock_gate_dis2 = I915_READ(VLV_GUNIT_CLOCK_GATE2);
+ s-dpio_cfg_data= I915_READ(DPIO_CTL);
/*
* Not saving any of:
@@ -1192,6 +1209,7 @@ static void vlv_restore_gunit_s0ix_state(struct
drm_i915_private *dev_priv)
I915_WRITE(VLV_GU_CTL0, s-gu_ctl0);
I915_WRITE(VLV_GU_CTL1, s-gu_ctl1);
I915_WRITE(VLV_GUNIT_CLOCK_GATE2, s-clock_gate_dis2);
+ I915_WRITE(DPIO_CTL,s-dpio_cfg_data);
}
int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
@@ -1291,6 +1309,8 @@ static void vlv_check_no_gt_access(struct
drm_i915_private *dev_priv)
I915_WRITE(VLV_GTLC_PW_STATUS, VLV_GTLC_ALLOWWAKEERR);
}
+/* This function is used in system suspend path as well to utilize
+ * Gfx clock, Wake control, Gunit state save related functionaility */
static int vlv_runtime_suspend(struct drm_i915_private *dev_priv)
{
u32 mask;
@@ -1331,7 +1351,12 @@ err1:
return err;
}
-static int vlv_runtime_resume(struct
Re: [Intel-gfx] [PATCH] drm/i915: FBC flush nuke for BDW
On Thu, Aug 07, 2014 at 01:04:19PM -0700, Rodrigo Vivi wrote:
I tested here on HSW a full sw nuke/cache clean and I didn't liked the
result.
It seems to compress less than the hw one and to recompress everything a
lot and stay less time compressed.
That is really unexpected. For a modern desktop (i.e. anything that
pageflips) there should be zero difference. And for actual frontbuffer
rendering there should only be a difference when doing tiny cpu rendering
to the frontbuffer.
So something didn't work out as expected. Can you please push the code
somewhere, or just submit patches to intel-gfx?
Thanks, Daniel
So, imho v3 is the way to go.
On Mon, Aug 4, 2014 at 3:51 AM, Rodrigo Vivi rodrigo.v...@intel.com wrote:
According to spec FBC on BDW and HSW are identical without any gaps.
So let's copy the nuke and let FBC really start compressing stuff.
Without this patch we can verify with false color that nothing is being
compressed. With the nuke in place and false color it is possible
to see false color debugs.
Unfortunatelly on some rings like BCS on BDW we have to avoid Bits 22:18 on
LRIs due to a high risk of hung. So, when using Blt ring for frontbuffer
rend
cache would never been cleaned and FBC would stop compressing buffer.
One alternative is to cache clean on software frontbuffer tracking.
v2: Fix rebase conflict.
v3: Do not clean cache on BCS ring. Instead use sw frontbuffer tracking.
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_display.c| 3 +++
drivers/gpu/drm/i915/intel_pm.c | 10 ++
drivers/gpu/drm/i915/intel_ringbuffer.c | 10 +-
4 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 2a372f2..25d7365 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2713,6 +2713,7 @@ extern void intel_modeset_setup_hw_state(struct
drm_device *dev,
extern void i915_redisable_vga(struct drm_device *dev);
extern void i915_redisable_vga_power_on(struct drm_device *dev);
extern bool intel_fbc_enabled(struct drm_device *dev);
+extern void gen8_fbc_sw_flush(struct drm_device *dev, u32 value);
extern void intel_disable_fbc(struct drm_device *dev);
extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
extern void intel_init_pch_refclk(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 883af0b..c8421cd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9044,6 +9044,9 @@ void intel_frontbuffer_flush(struct drm_device *dev,
intel_mark_fb_busy(dev, frontbuffer_bits, NULL);
intel_edp_psr_flush(dev, frontbuffer_bits);
+
+ if (IS_GEN8(dev))
+ gen8_fbc_sw_flush(dev, FBC_REND_CACHE_CLEAN);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 684dc5f..de07d3e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -345,6 +345,16 @@ bool intel_fbc_enabled(struct drm_device *dev)
return dev_priv-display.fbc_enabled(dev);
}
+void gen8_fbc_sw_flush(struct drm_device *dev, u32 value)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+
+ if (!IS_GEN8(dev))
+ return;
+
+ I915_WRITE(MSG_FBC_REND_STATE, value);
+}
+
static void intel_fbc_work_fn(struct work_struct *__work)
{
struct intel_fbc_work *work =
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2908896..2fe871c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -406,6 +406,7 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
{
u32 flags = 0;
u32 scratch_addr = ring-scratch.gtt_offset + 2 * CACHELINE_BYTES;
+ int ret;
flags |= PIPE_CONTROL_CS_STALL;
@@ -424,7 +425,14 @@ gen8_render_ring_flush(struct intel_engine_cs *ring,
flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
}
- return gen8_emit_pipe_control(ring, flags, scratch_addr);
+ ret = gen8_emit_pipe_control(ring, flags, scratch_addr);
+ if (ret)
+ return ret;
+
+ if (!invalidate_domains flush_domains)
+ return gen7_ring_fbc_flush(ring, FBC_REND_NUKE);
+
+ return 0;
}
static void ring_write_tail(struct intel_engine_cs *ring,
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
Re: [Intel-gfx] [PATCH 7/7] Revert drm/i915: Enable semaphores on BDW
On Thu, Aug 07, 2014 at 01:05:52PM -0700, Rodrigo Vivi wrote: The rest of the series was a reference for the records of what I had and let semaphores on bdw a bit more stable. But even with them we still get hungs so please consider only to get the revert for now. Thanks for the reminder, picked up for -fixes, thanks for the patch. -Daniel On Mon, Aug 4, 2014 at 11:15 AM, Rodrigo Vivi rodrigo.v...@intel.com wrote: This reverts commit 521e62e49a42661a4ee0102644517dbe2f100a23. Although POST_SYNC brought a bit of stability to Semaphores on BDW it didn't solved all issues and some hungs can still occour when semaphores are enabled on BDW. Also some sloweness can be found on some igt tests, althoguth it apparently doesn't affect real workloads. Besides that, no real performance gain was found on our tests with different and even multiple workloads. Let's disable it again for now. At least until we are sure it is safe to re-enable it. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6c4b25c..ec96f9a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -481,6 +481,10 @@ bool i915_semaphore_is_enabled(struct drm_device *dev) if (i915.semaphores = 0) return i915.semaphores; + /* Until we get further testing... */ + if (IS_GEN8(dev)) + return false; + #ifdef CONFIG_INTEL_IOMMU /* Enable semaphores on SNB when IO remapping is off */ if (INTEL_INFO(dev)-gen == 6 intel_iommu_gfx_mapped) -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/7] Revert drm/i915: Enable semaphores on BDW
On Mon, Aug 04, 2014 at 11:15:19AM -0700, Rodrigo Vivi wrote: This reverts commit 521e62e49a42661a4ee0102644517dbe2f100a23. Although POST_SYNC brought a bit of stability to Semaphores on BDW it didn't solved all issues and some hungs can still occour when semaphores are enabled on BDW. Also some sloweness can be found on some igt tests, althoguth it apparently doesn't affect real workloads. Besides that, no real performance gain was found on our tests with different and even multiple workloads. Let's disable it again for now. At least until we are sure it is safe to re-enable it. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 6c4b25c..ec96f9a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -481,6 +481,10 @@ bool i915_semaphore_is_enabled(struct drm_device *dev) if (i915.semaphores = 0) return i915.semaphores; + /* Until we get further testing... */ + if (IS_GEN8(dev)) + return false; Aside: With this we can't test the code any more at all. The usual approach for adjusting defaults when it's not the same on all platforms is to set the module option to -1 (per-platform defaults) and have a sanitize_foo function call to set it to the correct default. Would be nice on top of the revert. -Daniel + #ifdef CONFIG_INTEL_IOMMU /* Enable semaphores on SNB when IO remapping is off */ if (INTEL_INFO(dev)-gen == 6 intel_iommu_gfx_mapped) -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/7] Revert drm/i915: Enable semaphores on BDW
On Fri, Aug 08, 2014 at 09:13:06AM +0200, Daniel Vetter wrote: On Mon, Aug 04, 2014 at 11:15:19AM -0700, Rodrigo Vivi wrote: Besides that, no real performance gain was found on our tests with different and even multiple workloads. That either means you didn't try hard enough, or that the implementation is extremely inefficient. -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 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.
On Fri, Aug 08, 2014 at 12:22:44PM +0530, Sagar Arun Kamble wrote: Hi Daniel, On Mon, 2014-08-04 at 10:07 +0200, Daniel Vetter wrote: On Fri, Aug 01, 2014 at 12:34:56PM +0530, sagar.a.kam...@intel.com wrote: From: Sagar Kamble sagar.a.kam...@intel.com @@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev) intel_display_set_init_power(dev_priv, false); - return 0; + /* Save Gunit State and clear wake - Need to make sure + * changes in vlv_runtime_suspend path don't impact this path */ + if (IS_VALLEYVIEW(dev)) + ret = vlv_runtime_suspend(dev_priv); Maybe I wasn't clear, but I absolutely don't want any IS_VLV additions to core resume/thaw code. This should be shovelled into the runtime pm handling code, which should be reused in the suspend/resume code. This piece of code does not fit into any of the power well get/put path. Its specific sequence that need to be followed in VLV when Gunit gets power gated. So we have to keep this IS_VLV related functionality in both runtime and pm suspend/resume. Well we support S0ix now. Which means our system suspend/resume code actually calls into the runtime pm code. So either that design is broken (and we need to fix this) or something else is amiss. Or we don't need this code any more. But duplicating it is not the right approach. And yeah the power domain stuff might not be the right place, I've just used that as a place-holder for all the runtime pm code we have. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.
On Fri, 2014-08-08 at 09:42 +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 12:22:44PM +0530, Sagar Arun Kamble wrote: Hi Daniel, On Mon, 2014-08-04 at 10:07 +0200, Daniel Vetter wrote: On Fri, Aug 01, 2014 at 12:34:56PM +0530, sagar.a.kam...@intel.com wrote: From: Sagar Kamble sagar.a.kam...@intel.com @@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev) intel_display_set_init_power(dev_priv, false); - return 0; + /* Save Gunit State and clear wake - Need to make sure +* changes in vlv_runtime_suspend path don't impact this path */ + if (IS_VALLEYVIEW(dev)) + ret = vlv_runtime_suspend(dev_priv); Maybe I wasn't clear, but I absolutely don't want any IS_VLV additions to core resume/thaw code. This should be shovelled into the runtime pm handling code, which should be reused in the suspend/resume code. This piece of code does not fit into any of the power well get/put path. Its specific sequence that need to be followed in VLV when Gunit gets power gated. So we have to keep this IS_VLV related functionality in both runtime and pm suspend/resume. Well we support S0ix now. Which means our system suspend/resume code actually calls into the runtime pm code. So either that design is broken (and we need to fix this) or something else is amiss. Or we don't need this code any more. But duplicating it is not the right approach. And yeah the power domain stuff might not be the right place, I've just used that as a place-holder for all the runtime pm code we have. -Daniel While entering S0iX we are getting following call flow: intel_runtime_resume followed by i915_drm_freeze and While resuming back from S0iX: i915_drm_thaw How can we share that wake control, gunit save/restore functionality? I am observing issue given below: Here is how I am achieving S0ix: 1. echo mem /sys/power/state 2. (Assuming we have removed all wake_locks in /sys/powe/wake_lock) If device is runtime suspend it is getting resumed with runtime_resume 3. PM core triggers this sequence for each device (prepare, suspend, suspend_late, suspend_noirq) 4. Then pm_suspend happens for Gfx. After all devices are suspended S0i0-S0iR-S0i1-S0i2-S0i3 happens. Gunit gets power gated. While resuming back (with user action/power button press) 1. PM core triggers following sequenc for each device: (resume_noirq, resume_early, resume, complete) After call to i915_drm_thaw_early(called through resume_early), during i915_drm_thaw(called through resume) we were seeing following issue during ring initialization. This is happening since wake is not enabled and Gunit registers are not restored(which is done in runtime_resume but that path is not taken here since this is resume from pm_suspend) 6[ 3152.991399] PM: device[controlD64] driver[drm] resume enter 3[ 3153.976759] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring 3[ 3154.980061] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring 3[ 3154.980067] [drm:init_ring_common] *ERROR* failed to set render ring head to zero ctl head tail start 3[ 3155.983364] [drm:stop_ring] *ERROR* bsd ring :timed out trying to stop ring 3[ 3156.986668] [drm:stop_ring] *ERROR* bsd ring :timed out trying to stop ring 3[ 3156.986673] [drm:init_ring_common] *ERROR* failed to set bsd ring head to zero ctl head tail start 3[ 3157.989982] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring thanks, Sagar ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.
On Fri, Aug 08, 2014 at 02:29:38PM +0530, Sagar Arun Kamble wrote: On Fri, 2014-08-08 at 09:42 +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 12:22:44PM +0530, Sagar Arun Kamble wrote: Hi Daniel, On Mon, 2014-08-04 at 10:07 +0200, Daniel Vetter wrote: On Fri, Aug 01, 2014 at 12:34:56PM +0530, sagar.a.kam...@intel.com wrote: From: Sagar Kamble sagar.a.kam...@intel.com @@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev) intel_display_set_init_power(dev_priv, false); - return 0; + /* Save Gunit State and clear wake - Need to make sure + * changes in vlv_runtime_suspend path don't impact this path */ + if (IS_VALLEYVIEW(dev)) + ret = vlv_runtime_suspend(dev_priv); Maybe I wasn't clear, but I absolutely don't want any IS_VLV additions to core resume/thaw code. This should be shovelled into the runtime pm handling code, which should be reused in the suspend/resume code. This piece of code does not fit into any of the power well get/put path. Its specific sequence that need to be followed in VLV when Gunit gets power gated. So we have to keep this IS_VLV related functionality in both runtime and pm suspend/resume. Well we support S0ix now. Which means our system suspend/resume code actually calls into the runtime pm code. So either that design is broken (and we need to fix this) or something else is amiss. Or we don't need this code any more. But duplicating it is not the right approach. And yeah the power domain stuff might not be the right place, I've just used that as a place-holder for all the runtime pm code we have. -Daniel While entering S0iX we are getting following call flow: intel_runtime_resume followed by i915_drm_freeze and While resuming back from S0iX: i915_drm_thaw How can we share that wake control, gunit save/restore functionality? I am observing issue given below: Here is how I am achieving S0ix: 1. echo mem /sys/power/state 2. (Assuming we have removed all wake_locks in /sys/powe/wake_lock) If device is runtime suspend it is getting resumed with runtime_resume 3. PM core triggers this sequence for each device (prepare, suspend, suspend_late, suspend_noirq) 4. Then pm_suspend happens for Gfx. After all devices are suspended S0i0-S0iR-S0i1-S0i2-S0i3 happens. Gunit gets power gated. While resuming back (with user action/power button press) 1. PM core triggers following sequenc for each device: (resume_noirq, resume_early, resume, complete) After call to i915_drm_thaw_early(called through resume_early), during i915_drm_thaw(called through resume) we were seeing following issue during ring initialization. This is happening since wake is not enabled and Gunit registers are not restored(which is done in runtime_resume but that path is not taken here since this is resume from pm_suspend) We have intel_runtime_pm_get/put calls in the suspend/resume paths which should achieve this. Maybe they're not at the right place or not in the right ordering, but they're there. So on platforms with runtime pm support we _do_ call all the relevant runtime pm callbacks from system suspend/resume. -Daniel 6[ 3152.991399] PM: device[controlD64] driver[drm] resume enter 3[ 3153.976759] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring 3[ 3154.980061] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring 3[ 3154.980067] [drm:init_ring_common] *ERROR* failed to set render ring head to zero ctl head tail start 3[ 3155.983364] [drm:stop_ring] *ERROR* bsd ring :timed out trying to stop ring 3[ 3156.986668] [drm:stop_ring] *ERROR* bsd ring :timed out trying to stop ring 3[ 3156.986673] [drm:init_ring_common] *ERROR* failed to set bsd ring head to zero ctl head tail start 3[ 3157.989982] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.
On Fri, 2014-08-08 at 14:29 +0530, Sagar Arun Kamble wrote: On Fri, 2014-08-08 at 09:42 +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 12:22:44PM +0530, Sagar Arun Kamble wrote: Hi Daniel, On Mon, 2014-08-04 at 10:07 +0200, Daniel Vetter wrote: On Fri, Aug 01, 2014 at 12:34:56PM +0530, sagar.a.kam...@intel.com wrote: From: Sagar Kamble sagar.a.kam...@intel.com @@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev) intel_display_set_init_power(dev_priv, false); - return 0; + /* Save Gunit State and clear wake - Need to make sure + * changes in vlv_runtime_suspend path don't impact this path */ + if (IS_VALLEYVIEW(dev)) + ret = vlv_runtime_suspend(dev_priv); Maybe I wasn't clear, but I absolutely don't want any IS_VLV additions to core resume/thaw code. This should be shovelled into the runtime pm handling code, which should be reused in the suspend/resume code. This piece of code does not fit into any of the power well get/put path. Its specific sequence that need to be followed in VLV when Gunit gets power gated. So we have to keep this IS_VLV related functionality in both runtime and pm suspend/resume. Well we support S0ix now. Which means our system suspend/resume code actually calls into the runtime pm code. So either that design is broken (and we need to fix this) or something else is amiss. Or we don't need this code any more. But duplicating it is not the right approach. And yeah the power domain stuff might not be the right place, I've just used that as a place-holder for all the runtime pm code we have. -Daniel While entering S0iX we are getting following call flow: intel_runtime_resume followed by i915_drm_freeze and While resuming back from S0iX: i915_drm_thaw How can we share that wake control, gunit save/restore functionality? I am observing issue given below: Here is how I am achieving S0ix: 1. echo mem /sys/power/state 2. (Assuming we have removed all wake_locks in /sys/powe/wake_lock) If device is runtime suspend it is getting resumed with runtime_resume 3. PM core triggers this sequence for each device (prepare, suspend, suspend_late, suspend_noirq) 4. Then pm_suspend happens for Gfx. After all devices are suspended S0i0-S0iR-S0i1-S0i2-S0i3 happens. Gunit gets power gated. While resuming back (with user action/power button press) 1. PM core triggers following sequenc for each device: (resume_noirq, resume_early, resume, complete) After call to i915_drm_thaw_early(called through resume_early), during i915_drm_thaw(called through resume) we were seeing following issue during ring initialization. This is happening since wake is not enabled and Gunit registers are not restored(which is done in runtime_resume but that path is not taken here since this is resume from pm_suspend) Yes, this is correct. The DPM core takes a runtime PM reference before calling the system suspend handler. So you have to call any needed parts explicitly from the system suspend handler that is shared with the runtime PM code. But the platform checks could be contained all within the called common handler and you'd call this handler from the system suspend handler unconditionally. I think this would be closer what Daniel suggested. --Imre 6[ 3152.991399] PM: device[controlD64] driver[drm] resume enter 3[ 3153.976759] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring 3[ 3154.980061] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring 3[ 3154.980067] [drm:init_ring_common] *ERROR* failed to set render ring head to zero ctl head tail start 3[ 3155.983364] [drm:stop_ring] *ERROR* bsd ring :timed out trying to stop ring 3[ 3156.986668] [drm:stop_ring] *ERROR* bsd ring :timed out trying to stop ring 3[ 3156.986673] [drm:init_ring_common] *ERROR* failed to set bsd ring head to zero ctl head tail start 3[ 3157.989982] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring thanks, Sagar signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO
On Wed, Aug 06, 2014 at 10:39:16AM +0200, Daniel Vetter wrote: On Wed, Aug 06, 2014 at 09:12:32AM +0100, Chris Wilson wrote: On Wed, Aug 06, 2014 at 09:56:45AM +0200, Daniel Vetter wrote: On Tue, Aug 05, 2014 at 07:51:14AM -0700, Rodrigo Vivi wrote: From: Chris Wilson ch...@chris-wilson.co.uk If we try to execute on a known ring, but it has failed to be initialised correctly, report that the GPU is hung rather than the command invalid. This leaves us reporting EINVAL only if the user requests execution on a ring that is not supported by the device. This should prevent UXA from getting stuck in a null render loop after a failed resume. v2 (Rodrigo): Fix conflict and add VCS2 ring and s/intel_ring_buffer/intel_engine_cs. Reported-by: Jiri Kosina ji...@jikos.cz References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com This isn't required any more, see commit 074c6adaf4e7d1423d373bd5d1afc20b683cb4d0 Author: Chris Wilson ch...@chris-wilson.co.uk Date: Wed Apr 9 09:19:43 2014 +0100 drm/i915: Mark device as wedged if we fail to resume for the alternate merged patch. Hmm, there is still a path that ends here, but the example above is already fixed as you say. We have the EIO check both in the resume and driver load paths. Which other path are we missing? The GPU may be set to wedged, but this check in execbuffer occurs before we check for a wedged 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 45/53] drm/i915/bdw: Do not call intel_runtime_pm_get() in an interrupt
On Tue, Jul 29, 2014 at 12:26:36PM +0200, Daniel Vetter wrote: On Tue, Jul 29, 2014 at 08:37:48AM +0100, Chris Wilson wrote: On Mon, Jul 28, 2014 at 10:54:06AM +0200, Daniel Vetter wrote: On Sat, Jul 26, 2014 at 11:27:38AM +0100, Chris Wilson wrote: On Wed, Jun 18, 2014 at 10:54:13PM +0200, Daniel Vetter wrote: On Fri, Jun 13, 2014 at 04:38:03PM +0100, oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com Or with a spinlock grabbed, because it might sleep, which is not a nice thing to do. Instead, do the runtime_pm get/put together with the create/destroy request, and handle the forcewake get/put directly. Signed-off-by: Oscar Mateo oscar.ma...@intel.com Looks like a fixup that should be squashed into relevant earlier patches. The whole gen6_gt_force_wake_get() calling intel_runtime_pm_get() is broken due to this - we must be able to read registers in atomic context! Please revert c8c8fb33b37766acf6474784b0d5245dab9a1690 force_wake_get can't call runtime_pm_get becuase pm_get can sleep. So if you want to read registers from atomic context you have to have a runtime pm reference from someone else. Nope. That cannot work. Well it works currently. So where do you see the problem? Sampling registers from an timer - in particular, we really do not want to disable runtime pm whilst trying to monitor the impact of runtime pm. -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] drm/i915: Make the physical object coherent with GTT
On Thu, Jul 10, 2014 at 09:53:43PM +0100, Chris Wilson wrote:
Currently objects for which the hardware needs a contiguous physical
address are allocated a shadow backing storage to satisfy the contraint.
This shadow buffer is not wired into the normal obj-pages and so the
physical object is incoherent with accesses via the GPU, GTT and CPU. By
setting up the appropriate scatter-gather table, we can allow userspace
to access the physical object via either a GTT mmaping of or by rendering
into the GEM bo. However, keeping the CPU mmap of the shmemfs backing
storage coherent with the contiguous shadow is not yet possible.
Fortuituously, CPU mmaps of objects requiring physical addresses are not
expected to be coherent anyway.
This allows the physical constraint of the GEM object to be transparent
to userspace and allow it to efficiently render into or update them via
the GTT and GPU.
v2: Fix leak of pci handle spotted by Ville
v3: Remove the now duplicate call to detach_phys_object during free.
v4: Wait for rendering before pwrite. As this patch makes it possible to
render into the phys object, we should make it correct as well!
Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Ok, I guess you can make a case that this fixes a bug, but then the commit
message should talk about that instead of only mentioning phys object
coherency (which is kinda just a side-effect of the rework to track phys
objects in the sg tables like everything else).
Plus mentioning that kms_cursors_crc provokes this if people bother to
plug in a vga connector or something.
With that I'll pull it into -fixes with a cc: stable without requiring
testcase for the coherency part.
-Daniel
---
drivers/gpu/drm/i915/i915_dma.c | 3 +
drivers/gpu/drm/i915/i915_drv.h | 6 +-
drivers/gpu/drm/i915/i915_gem.c | 207
+++-
include/uapi/drm/i915_drm.h | 1 +
4 files changed, 150 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 5ae608121f03..1b9798503b77 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1025,6 +1025,9 @@ static int i915_getparam(struct drm_device *dev, void
*data,
case I915_PARAM_CMD_PARSER_VERSION:
value = i915_cmd_parser_get_version();
break;
+ case I915_PARAM_HAS_COHERENT_PHYS_GTT:
+ value = 1;
+ break;
default:
DRM_DEBUG(Unknown parameter %d\n, param-param);
return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 211db0653831..f81d787d6b47 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1787,10 +1787,10 @@ struct drm_i915_gem_object {
unsigned long user_pin_count;
struct drm_file *pin_filp;
- /** for phy allocated objects */
- drm_dma_handle_t *phys_handle;
-
union {
+ /** for phy allocated objects */
+ drm_dma_handle_t *phys_handle;
+
struct i915_gem_userptr {
uintptr_t ptr;
unsigned read_only :1;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 83ccbabdcacf..d083cf5bdbbd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -209,40 +209,137 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev,
void *data,
return 0;
}
-static void i915_gem_object_detach_phys(struct drm_i915_gem_object *obj)
+static int
+i915_gem_object_get_pages_phys(struct drm_i915_gem_object *obj)
{
- drm_dma_handle_t *phys = obj-phys_handle;
+ struct address_space *mapping = file_inode(obj-base.filp)-i_mapping;
+ char *vaddr = obj-phys_handle-vaddr;
+ struct sg_table *st;
+ struct scatterlist *sg;
+ int i;
- if (!phys)
- return;
+ if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
+ return -EINVAL;
+
+ for (i = 0; i obj-base.size / PAGE_SIZE; i++) {
+ struct page *page;
+ char *src;
+
+ page = shmem_read_mapping_page(mapping, i);
+ if (IS_ERR(page))
+ return PTR_ERR(page);
+
+ src = kmap_atomic(page);
+ memcpy(vaddr, src, PAGE_SIZE);
+ drm_clflush_virt_range(vaddr, PAGE_SIZE);
+ kunmap_atomic(src);
+
+ page_cache_release(page);
+ vaddr += PAGE_SIZE;
+ }
+
+ i915_gem_chipset_flush(obj-base.dev);
+
+ st = kmalloc(sizeof(*st), GFP_KERNEL);
+ if (st == NULL)
+ return -ENOMEM;
+
+ if (sg_alloc_table(st, 1, GFP_KERNEL)) {
+ kfree(st);
+ return -ENOMEM;
+ }
+
+
Re: [Intel-gfx] [PATCH 45/53] drm/i915/bdw: Do not call intel_runtime_pm_get() in an interrupt
On Fri, Aug 08, 2014 at 10:20:40AM +0100, Chris Wilson wrote:
On Tue, Jul 29, 2014 at 12:26:36PM +0200, Daniel Vetter wrote:
On Tue, Jul 29, 2014 at 08:37:48AM +0100, Chris Wilson wrote:
On Mon, Jul 28, 2014 at 10:54:06AM +0200, Daniel Vetter wrote:
On Sat, Jul 26, 2014 at 11:27:38AM +0100, Chris Wilson wrote:
On Wed, Jun 18, 2014 at 10:54:13PM +0200, Daniel Vetter wrote:
On Fri, Jun 13, 2014 at 04:38:03PM +0100, oscar.ma...@intel.com
wrote:
From: Oscar Mateo oscar.ma...@intel.com
Or with a spinlock grabbed, because it might sleep, which is not
a nice thing to do. Instead, do the runtime_pm get/put together
with the create/destroy request, and handle the forcewake get/put
directly.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
Looks like a fixup that should be squashed into relevant earlier
patches.
The whole gen6_gt_force_wake_get() calling intel_runtime_pm_get() is
broken due to this - we must be able to read registers in atomic
context!
Please revert c8c8fb33b37766acf6474784b0d5245dab9a1690
force_wake_get can't call runtime_pm_get becuase pm_get can sleep. So if
you want to read registers from atomic context you have to have a
runtime
pm reference from someone else.
Nope. That cannot work.
Well it works currently. So where do you see the problem?
Sampling registers from an timer - in particular, we really do not want
to disable runtime pm whilst trying to monitor the impact of runtime pm.
In that case you can grab a runtime pm reference iff the device is powered
on already. Which won't call anything scary, just amounts to an
atomic_add_unless or so, and then drop it again.
Unfortunately there doesn't seem to be such a thing around already, so
need to add it first. Greg, how much would you freak out if we add
something like
/**
* pm_runtime_get_unless_suspended - grab a rpm ref if the device is on
*
* Returns true if an rpm ref has been acquire, false otherwise. Can be
* called from atomic context to e.g. sample perfomance counters (where we
* obviously don't want to disturb system state if everything is off atm).
*/
static inline bool pm_runtime_get_unless_suspended(struct device *dev)
{
return atomic_add_unless(dev-power.usage_count, 1, 0);
}
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] PROBLEM: Native backlight regressed from logarithmic to linear scale
On Tue, 29 Jul 2014, Daniel Vetter dan...@ffwll.ch wrote: On Tue, Jul 29, 2014 at 06:14:16AM -0400, Anders Kaseorg wrote: On Tue, 29 Jul 2014, Hans de Goede wrote: I've been thinking a bit about this, and I believe that the right answer here is to do the linear to logarithmic mapping in user-space. The intel backlight interface has a type of raw, clearly signalling to userspace that it is a raw untranslated interface, as such any fanciness such as creating a logarithmic scale should be done in userspace IMHO. I was going to respond that the kernel does its own brightness stepping when userspace isn’t paying attention. But apparently only acpi_video does that, and intel_backlight does not; my brightness keys now have no effect outside of the X server. Is that the expected behavior? Userspace on linux is supposed to catch brightness keys and update the backlight. Some acpi drivers do funny stuff behind everyone's back, but generally that's the expected behavior. You'd need a deamon for the backlight to work on the console. In any case, if you think punting part of the problem to userspace is the right answer, then to flesh out the details: do you think it’s right for userspace to assume that any backlight with type ‘raw’ is a linear scale that needs to be converted, and one with type ‘firmware’ or ‘platform’ has already been converted appropriately? I don't think you can generally assume anything - what we do is send the pwm signal, how linearly that translates into brightness is totally panel and driver dependent. So no matter what you pick someone will complain I think. Because the mapping from PWM duty cycle to luminance is panel dependent, the ACPI opregion contains such mapping. Likely the ACPI backlight uses just that. We (i915) currently don't. I don't think the userspace has a sensible interface to that information. I'm not sure it should either. I haven't made up my mind on this, but I might go for doing the mapping in i915. Additionally I think we should probably use a fixed range of, say, 0-100 that gets exposed to the userspace; there's no point in exposing e.g. 10 levels when the hardware can not physically produce nor can the user distinguish that many distinct levels. I'd go for making this as simple as possible to use and implement right. Anything fancy is going to blow up in fantastic ways. 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 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO
On Fri, Aug 08, 2014 at 10:17:10AM +0100, Chris Wilson wrote: On Wed, Aug 06, 2014 at 10:39:16AM +0200, Daniel Vetter wrote: On Wed, Aug 06, 2014 at 09:12:32AM +0100, Chris Wilson wrote: On Wed, Aug 06, 2014 at 09:56:45AM +0200, Daniel Vetter wrote: On Tue, Aug 05, 2014 at 07:51:14AM -0700, Rodrigo Vivi wrote: From: Chris Wilson ch...@chris-wilson.co.uk If we try to execute on a known ring, but it has failed to be initialised correctly, report that the GPU is hung rather than the command invalid. This leaves us reporting EINVAL only if the user requests execution on a ring that is not supported by the device. This should prevent UXA from getting stuck in a null render loop after a failed resume. v2 (Rodrigo): Fix conflict and add VCS2 ring and s/intel_ring_buffer/intel_engine_cs. Reported-by: Jiri Kosina ji...@jikos.cz References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com This isn't required any more, see commit 074c6adaf4e7d1423d373bd5d1afc20b683cb4d0 Author: Chris Wilson ch...@chris-wilson.co.uk Date: Wed Apr 9 09:19:43 2014 +0100 drm/i915: Mark device as wedged if we fail to resume for the alternate merged patch. Hmm, there is still a path that ends here, but the example above is already fixed as you say. We have the EIO check both in the resume and driver load paths. Which other path are we missing? The GPU may be set to wedged, but this check in execbuffer occurs before we check for a wedged GPU. But we no longer free the ring structures over suspedn/resume, so at least the commit message is outdated. I wonder whether the easier fix wouldn't be to continue ring init if we get an -EIO. -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] 3.16.0: [drm:i915_gem_init] *ERROR* Failed to initialize GPU, declaring it wedged
Re-adding mailing lists, please don't drop ccs like that. On Fri, Aug 8, 2014 at 11:45 AM, Luuk van der Duim luukvanderd...@gmail.com wrote: I'll be happy to test your duct-tape. ;) What branch do I need to pull? (what is its git location address?) git://anongit.freedesktop.org/drm-intel drm-intel-fixes Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/15] drm/i915: Upgrade execbuffer fail after resume failure to EIO
On Fri, Aug 08, 2014 at 11:46:07AM +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 10:17:10AM +0100, Chris Wilson wrote: On Wed, Aug 06, 2014 at 10:39:16AM +0200, Daniel Vetter wrote: On Wed, Aug 06, 2014 at 09:12:32AM +0100, Chris Wilson wrote: On Wed, Aug 06, 2014 at 09:56:45AM +0200, Daniel Vetter wrote: On Tue, Aug 05, 2014 at 07:51:14AM -0700, Rodrigo Vivi wrote: From: Chris Wilson ch...@chris-wilson.co.uk If we try to execute on a known ring, but it has failed to be initialised correctly, report that the GPU is hung rather than the command invalid. This leaves us reporting EINVAL only if the user requests execution on a ring that is not supported by the device. This should prevent UXA from getting stuck in a null render loop after a failed resume. v2 (Rodrigo): Fix conflict and add VCS2 ring and s/intel_ring_buffer/intel_engine_cs. Reported-by: Jiri Kosina ji...@jikos.cz References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com This isn't required any more, see commit 074c6adaf4e7d1423d373bd5d1afc20b683cb4d0 Author: Chris Wilson ch...@chris-wilson.co.uk Date: Wed Apr 9 09:19:43 2014 +0100 drm/i915: Mark device as wedged if we fail to resume for the alternate merged patch. Hmm, there is still a path that ends here, but the example above is already fixed as you say. We have the EIO check both in the resume and driver load paths. Which other path are we missing? The GPU may be set to wedged, but this check in execbuffer occurs before we check for a wedged GPU. But we no longer free the ring structures over suspedn/resume, so at least the commit message is outdated. Went off on an incorrect tangent. I wonder whether the easier fix wouldn't be to continue ring init if we get an -EIO. Right, that's the problem I remember. But I am not sure if it is really worth it. It's only to hide log spew in a corner case of a corner case in UXA. -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] [RFC] Add BDW workarounds to golden render state
From: Arun Siluvery arun.siluv...@linux.intel.com In this patch workarounds for BDW are applied to golden render state. Only those registers that are part of register state are added to this batch. Remaining workarounds are still in its current place init_clock_gating() which are not affected by a gpu reset. I can send another patch where they can be moved to render ring init function but during testing I found their state doesn't change after reset, please give your comments on this. Arun Siluvery (1): drm/i915/bdw: Apply workarounds to the golden render state drivers/gpu/drm/i915/intel_pm.c | 50 - drivers/gpu/drm/i915/intel_renderstate_gen8.c | 62 +-- 2 files changed, 39 insertions(+), 73 deletions(-) -- 2.0.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC] drm/i915/bdw: Apply workarounds to the golden render state
From: Arun Siluvery arun.siluv...@linux.intel.com
Workarounds for bdw are currently applied in init_clock_gating() but they
are lost following a gpu reset. Some of the registers are part of register
state context and they are restored with every context switch so initializing
WAs in golden render state ensures that they are applied even when we start
with an uninitialized context or during hw initialization followed by a reset.
Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
drivers/gpu/drm/i915/intel_pm.c | 50 -
drivers/gpu/drm/i915/intel_renderstate_gen8.c | 62 +--
2 files changed, 39 insertions(+), 73 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1ddd4df..ab64b64 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5402,38 +5402,11 @@ static void gen8_init_clock_gating(struct drm_device
*dev)
/* FIXME(BDW): Check all the w/a, some might only apply to
* pre-production hw. */
- /* WaDisablePartialInstShootdown:bdw */
- I915_WRITE(GEN8_ROW_CHICKEN,
- _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE));
-
- /* WaDisableThreadStallDopClockGating:bdw */
- /* FIXME: Unclear whether we really need this on production bdw. */
- I915_WRITE(GEN8_ROW_CHICKEN,
- _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE));
-
- /*
-* This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for
-* pre-production hardware
-*/
- I915_WRITE(HALF_SLICE_CHICKEN3,
- _MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS));
- I915_WRITE(HALF_SLICE_CHICKEN3,
- _MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS));
I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_BWGTLB_DISABLE));
I915_WRITE(_3D_CHICKEN3,
_MASKED_BIT_ENABLE(_3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(2)));
- I915_WRITE(COMMON_SLICE_CHICKEN2,
- _MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE));
-
- I915_WRITE(GEN7_HALF_SLICE_CHICKEN1,
- _MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE));
-
- /* WaDisableDopClockGating:bdw May not be needed for production */
- I915_WRITE(GEN7_ROW_CHICKEN2,
- _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE));
-
/* WaSwitchSolVfFArbitrationPriority:bdw */
I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL);
@@ -5448,41 +5421,18 @@ static void gen8_init_clock_gating(struct drm_device
*dev)
BDW_DPRS_MASK_VBLANK_SRD);
}
- /* Use Force Non-Coherent whenever executing a 3D context. This is a
-* workaround for for a possible hang in the unlikely event a TLB
-* invalidation occurs during a PSD flush.
-*/
- I915_WRITE(HDC_CHICKEN0,
- I915_READ(HDC_CHICKEN0) |
- _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT));
-
/* WaVSRefCountFullforceMissDisable:bdw */
/* WaDSRefCountFullforceMissDisable:bdw */
I915_WRITE(GEN7_FF_THREAD_MODE,
I915_READ(GEN7_FF_THREAD_MODE)
~(GEN8_FF_DS_REF_CNT_FFME | GEN7_FF_VS_REF_CNT_FFME));
- /*
-* BSpec recommends 8x4 when MSAA is used,
-* however in practice 16x4 seems fastest.
-*
-* Note that PS/WM thread counts depend on the WIZ hashing
-* disable bit, which we don't touch here, but it's good
-* to keep in mind (see 3DSTATE_PS and 3DSTATE_WM).
-*/
- I915_WRITE(GEN7_GT_MODE,
- GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
-
I915_WRITE(GEN6_RC_SLEEP_PSMI_CONTROL,
_MASKED_BIT_ENABLE(GEN8_RC_SEMA_IDLE_MSG_DISABLE));
/* WaDisableSDEUnitClockGating:bdw */
I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
-
- /* Wa4x4STCOptimizationDisable:bdw */
- I915_WRITE(CACHE_MODE_1,
- _MASKED_BIT_ENABLE(GEN8_4x4_STC_OPTIMIZATION_DISABLE));
}
static void haswell_init_clock_gating(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen8.c
b/drivers/gpu/drm/i915/intel_renderstate_gen8.c
index 75ef1b5..0b26783 100644
--- a/drivers/gpu/drm/i915/intel_renderstate_gen8.c
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen8.c
@@ -1,14 +1,38 @@
#include intel_renderstate.h
static const u32 gen8_null_state_relocs[] = {
- 0x0048,
- 0x0050,
- 0x0060,
- 0x03ec,
+ 0x00a8,
+ 0x00b0,
+ 0x00c0,
+ 0x044c,
-1,
};
static const u32 gen8_null_state_batch[] = {
+ 0x1101,
+ 0xe4f0,
+ 0x83208320,
+ 0x1101,
+ 0xe4f4,
+ 0x00010001,
+ 0x1101,
+
[Intel-gfx] [RFC 1/2] tools/null_state_render: Add BDW workarounds to golden render state
From: Arun Siluvery arun.siluv...@linux.intel.com
Some workaround registers are part of register state context and they are
restored with every context switch so initializing them in golden render state
ensures that they are applied even when we start with an uninitialized context
or during hw initialization followed by a reset.
Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
lib/intel_reg.h | 22
tools/null_state_gen/intel_renderstate_gen8.c | 37 +++
2 files changed, 59 insertions(+)
diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index 4afec45..86175bb 100644
--- a/lib/intel_reg.h
+++ b/lib/intel_reg.h
@@ -3606,4 +3606,26 @@ typedef enum {
#define GEN7_ROW_CHICKEN2_GT2 0xf4f4
#define DOP_CLOCK_GATING_DISABLE (10)
+#define HALF_SLICE_CHICKEN30xe184
+#define GEN8_CENTROID_PIXEL_OPT_DIS (18)
+#define GEN8_SAMPLER_POWER_BYPASS_DIS(11)
+
+#define GEN7_HALF_SLICE_CHICKEN1 0xe100
+#define GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE (110)
+
+#define COMMON_SLICE_CHICKEN2 0x7014
+# define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE (10)
+
+/* GEN8 chicken */
+#define HDC_CHICKEN0 0x7300
+#define HDC_FORCE_NON_COHERENT(14)
+
+#define GEN7_CACHE_MODE_1 0x7004 /* IVB+ */
+#define GEN8_4x4_STC_OPTIMIZATION_DISABLE(16)
+
+#define GEN7_GT_MODE 0x7008
+#define GEN6_WIZ_HASHING(hi, lo) (((hi) 9) | ((lo) 7))
+#define GEN6_WIZ_HASHING_16x4GEN6_WIZ_HASHING(1, 0)
+#define GEN6_WIZ_HASHING_MASK(GEN6_WIZ_HASHING(1, 1)
16)
+
#endif /* _I810_REG_H */
diff --git a/tools/null_state_gen/intel_renderstate_gen8.c
b/tools/null_state_gen/intel_renderstate_gen8.c
index 14690d2..a8143db 100644
--- a/tools/null_state_gen/intel_renderstate_gen8.c
+++ b/tools/null_state_gen/intel_renderstate_gen8.c
@@ -703,6 +703,43 @@ static void gen8_emit_workarounds(struct intel_batchbuffer
*batch)
/* WaDisableDopClockGating:bdw May not be needed for production */
gen8_emit_write(batch, GEN7_ROW_CHICKEN2,
_MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE));
+
+ /*
+* This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for
+* pre-production hardware
+*/
+ gen8_emit_write(batch, HALF_SLICE_CHICKEN3,
+ _MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS
+ | GEN8_SAMPLER_POWER_BYPASS_DIS));
+
+ gen8_emit_write(batch, GEN7_HALF_SLICE_CHICKEN1,
+
_MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE));
+
+ gen8_emit_write(batch, COMMON_SLICE_CHICKEN2,
+
_MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE));
+
+ /* Use Force Non-Coherent whenever executing a 3D context. This is a
+* workaround for for a possible hang in the unlikely event a TLB
+* invalidation occurs during a PSD flush.
+*/
+ gen8_emit_write(batch, HDC_CHICKEN0,
+ _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT));
+
+ /* Wa4x4STCOptimizationDisable:bdw */
+ gen8_emit_write(batch, GEN7_CACHE_MODE_1,
+ _MASKED_BIT_ENABLE(GEN8_4x4_STC_OPTIMIZATION_DISABLE));
+
+ /*
+* BSpec recommends 8x4 when MSAA is used,
+* however in practice 16x4 seems fastest.
+*
+* Note that PS/WM thread counts depend on the WIZ hashing
+* disable bit, which we don't touch here, but it's good
+* to keep in mind (see 3DSTATE_PS and 3DSTATE_WM).
+*/
+ gen8_emit_write(batch, GEN7_GT_MODE,
+ GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4);
+
}
int gen8_setup_null_render_state(struct intel_batchbuffer *batch)
--
2.0.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 0/2] Add test to verify BDW workarounds
From: Arun Siluvery arun.siluv...@linux.intel.com Currently in BDW workarounds are initialized in init_clock_gating() but some of them are lost followed by a gpu reset. The solution is to apply them in golden render state which keeps them valid when starting with an uninitialized state. First patch adds workaround registers to golden render state. This patch uses functions which are part two other patches, one of which is not yet sent for upstream [2]. [1] https://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg43044.html [2] From cc8f2f657c44d7c017e70e1af11b4513358b6a0a Mon Sep 17 00:00:00 2001 From: Deepak S deepa...@linux.intel.com Date: Sat, 2 Aug 2014 20:05:07 +0530 Subject: [PATCH 2/3] tools/null_state_render: Add ROW CHICKEN BIT Registers to golden render state As per, latest Bspec, some of the RenderCS registers are expected to be added in context. We achive this by adding the registers Golden render state. Signed-off-by: Deepak S deepa...@linux.intel.com Second patch adds a simple test which captures workaround register data before and after a gpu reset and compares them. Suspend/Resume is currently not working hence it is not tested. The test uses intel_reg_read() to read register data but I observed that this value is different from the one compared to using I915_READ() from within the driver. e.g., in case of CACHE_MODE_1 with WA applied it should be 0x01C0. intel_reg_read() gives 0x0180 whereas with I915_READ() it is 0x01C0. Arun Siluvery (2): tools/null_state_render: Add BDW workarounds to golden render state igt/gem_workarounds: igt to test workaround registers lib/intel_reg.h | 30 tests/Makefile.sources| 1 + tests/gem_workarounds.c | 211 ++ tools/null_state_gen/intel_renderstate_gen8.c | 37 + 4 files changed, 279 insertions(+) create mode 100644 tests/gem_workarounds.c -- 2.0.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [RFC 2/2] igt/gem_workarounds: igt to test workaround registers
From: Arun Siluvery arun.siluv...@linux.intel.com
Some of the workarounds are lost followed by a gpu reset, suspend/resume;
this patch adds a test which captures register state before and after
the test scenario.
This test currently verifies only bdw workarounds.
Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
lib/intel_reg.h | 8 ++
tests/Makefile.sources | 1 +
tests/gem_workarounds.c | 211
3 files changed, 220 insertions(+)
create mode 100644 tests/gem_workarounds.c
diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index 86175bb..d015c36 100644
--- a/lib/intel_reg.h
+++ b/lib/intel_reg.h
@@ -3628,4 +3628,12 @@ typedef enum {
#define GEN6_WIZ_HASHING_16x4GEN6_WIZ_HASHING(1, 0)
#define GEN6_WIZ_HASHING_MASK(GEN6_WIZ_HASHING(1, 1)
16)
+#define GAMTARBMODE0x04a08
+#define _3D_CHICKEN3 0x02090
+#define GAM_ECOCHK 0x4090
+#define CHICKEN_PAR1_1 0x42080
+#define GEN7_FF_THREAD_MODE0x20a0
+#define GEN6_RC_SLEEP_PSMI_CONTROL 0x2050
+#define GEN8_UCGCTL6 0x9430
+
#endif /* _I810_REG_H */
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 0eb9369..a17acd1 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -134,6 +134,7 @@ TESTS_progs = \
gem_unfence_active_buffers \
gem_unref_active_buffers \
gem_wait_render_timeout \
+ gem_workarounds \
gen3_mixed_blits \
gen3_render_linear_blits \
gen3_render_mixed_blits \
diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
new file mode 100644
index 000..35d1aa7
--- /dev/null
+++ b/tests/gem_workarounds.c
@@ -0,0 +1,211 @@
+/*
+ * Copyright ?? 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ * Arun Siluvery arun.siluv...@linux.intel.com
+ *
+ */
+
+#define _GNU_SOURCE
+#include stdbool.h
+#include unistd.h
+#include stdlib.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include sys/stat.h
+#include sys/ioctl.h
+#include sys/mman.h
+#include time.h
+#include signal.h
+
+#include ioctl_wrappers.h
+#include drmtest.h
+#include igt_debugfs.h
+#include igt_aux.h
+#include intel_chipset.h
+#include intel_io.h
+
+int drm_fd;
+static drm_intel_bufmgr *bufmgr;
+struct intel_batchbuffer *batch;
+uint32_t devid;
+
+enum operation {
+ GPU_RESET,
+ SUSPEND_RESUME,
+};
+
+struct workaround {
+ const char *reg_name;
+ uint32_t address;
+};
+
+static struct workaround bdw_workarounds[] =
+{
+ { GEN8_ROW_CHICKEN, GEN8_ROW_CHICKEN },
+ { GEN7_ROW_CHICKEN2, GEN7_ROW_CHICKEN2 },
+ { HALF_SLICE_CHICKEN3, HALF_SLICE_CHICKEN3 },
+ { GEN7_HALF_SLICE_CHICKEN1, GEN7_HALF_SLICE_CHICKEN1 },
+ { COMMON_SLICE_CHICKEN2, COMMON_SLICE_CHICKEN2 },
+ { HDC_CHICKEN0, HDC_CHICKEN0 },
+ { GEN7_CACHE_MODE_1, GEN7_CACHE_MODE_1 },
+ { GEN7_GT_MODE, GEN7_GT_MODE },
+ { GAMTARBMODE, GAMTARBMODE },
+ { _3D_CHICKEN3, _3D_CHICKEN3 },
+ { GAM_ECOCHK, GAM_ECOCHK },
+ { CHICKEN_PAR1_1, CHICKEN_PAR1_1 },
+ { GEN7_FF_THREAD_MODE, GEN7_FF_THREAD_MODE },
+ { GEN6_RC_SLEEP_PSMI_CONTROL, GEN6_RC_SLEEP_PSMI_CONTROL },
+ { GEN8_UCGCTL6, GEN8_UCGCTL6 },
+ { NULL, 0x },
+};
+
+static void test_hang_gpu(void)
+{
+ int retry_count = 30;
+ enum stop_ring_flags flags;
+ struct drm_i915_gem_execbuffer2 execbuf;
+ struct drm_i915_gem_exec_object2 gem_exec;
+ uint32_t b[2] = {MI_BATCH_BUFFER_END};
+
+ igt_assert(retry_count);
+ igt_set_stop_rings(STOP_RING_DEFAULTS);
+
+ memset(gem_exec, 0, sizeof(gem_exec));
+ gem_exec.handle = gem_create(drm_fd, 4096);
+
Re: [Intel-gfx] [RFC] drm/i915/bdw: Apply workarounds to the golden render state
On Fri, Aug 08, 2014 at 10:52:57AM +0100, arun.siluv...@linux.intel.com wrote: From: Arun Siluvery arun.siluv...@linux.intel.com Workarounds for bdw are currently applied in init_clock_gating() but they are lost following a gpu reset. Some of the registers are part of register state context and they are restored with every context switch so initializing WAs in golden render state ensures that they are applied even when we start with an uninitialized context or during hw initialization followed by a reset. Interesting, but let's try to keep the opaque blobs minimal. The comments for w/a are even more valuable than the code. -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 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.
On Fri, 2014-08-08 at 11:15 +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 02:29:38PM +0530, Sagar Arun Kamble wrote: On Fri, 2014-08-08 at 09:42 +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 12:22:44PM +0530, Sagar Arun Kamble wrote: Hi Daniel, On Mon, 2014-08-04 at 10:07 +0200, Daniel Vetter wrote: On Fri, Aug 01, 2014 at 12:34:56PM +0530, sagar.a.kam...@intel.com wrote: From: Sagar Kamble sagar.a.kam...@intel.com @@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev) intel_display_set_init_power(dev_priv, false); - return 0; + /* Save Gunit State and clear wake - Need to make sure +* changes in vlv_runtime_suspend path don't impact this path */ + if (IS_VALLEYVIEW(dev)) + ret = vlv_runtime_suspend(dev_priv); Maybe I wasn't clear, but I absolutely don't want any IS_VLV additions to core resume/thaw code. This should be shovelled into the runtime pm handling code, which should be reused in the suspend/resume code. This piece of code does not fit into any of the power well get/put path. Its specific sequence that need to be followed in VLV when Gunit gets power gated. So we have to keep this IS_VLV related functionality in both runtime and pm suspend/resume. Well we support S0ix now. Which means our system suspend/resume code actually calls into the runtime pm code. So either that design is broken (and we need to fix this) or something else is amiss. Or we don't need this code any more. But duplicating it is not the right approach. And yeah the power domain stuff might not be the right place, I've just used that as a place-holder for all the runtime pm code we have. -Daniel While entering S0iX we are getting following call flow: intel_runtime_resume followed by i915_drm_freeze and While resuming back from S0iX: i915_drm_thaw How can we share that wake control, gunit save/restore functionality? I am observing issue given below: Here is how I am achieving S0ix: 1. echo mem /sys/power/state 2. (Assuming we have removed all wake_locks in /sys/powe/wake_lock) If device is runtime suspend it is getting resumed with runtime_resume 3. PM core triggers this sequence for each device (prepare, suspend, suspend_late, suspend_noirq) 4. Then pm_suspend happens for Gfx. After all devices are suspended S0i0-S0iR-S0i1-S0i2-S0i3 happens. Gunit gets power gated. While resuming back (with user action/power button press) 1. PM core triggers following sequenc for each device: (resume_noirq, resume_early, resume, complete) After call to i915_drm_thaw_early(called through resume_early), during i915_drm_thaw(called through resume) we were seeing following issue during ring initialization. This is happening since wake is not enabled and Gunit registers are not restored(which is done in runtime_resume but that path is not taken here since this is resume from pm_suspend) We have intel_runtime_pm_get/put calls in the suspend/resume paths which should achieve this. Maybe they're not at the right place or not in the right ordering, but they're there. So on platforms with runtime pm support we _do_ call all the relevant runtime pm callbacks from system suspend/resume. -Daniel I am seeing following commit removed get/put calls from suspend resume paths: commit 395a5abbd97d7d76a7a26da52f33daebe279b3b3 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Wed Jun 18 09:52:56 2014 -0700 drm/i915: don't take runtime PM reference around freeze/thaw That ordering of get/put was also incorrect probably. Even though we do rpm_get before freeze after S0i3 we need to explicitely do wake contro/gunit save restore. In the current patch, the rpm handlers are called directly instead of (intel_runtime_pm_get/put). valleyview_runtime_suspend at the end of i915_drm_freeze and valleyview_runtime_resume at the beginning of i915_drm_thaw_early. Do I need to replace them with intel_display_power_get/put? Will DPM core allow rpm calls when system pm suspend/resume is in progress? 6[ 3152.991399] PM: device[controlD64] driver[drm] resume enter 3[ 3153.976759] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring 3[ 3154.980061] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring 3[ 3154.980067] [drm:init_ring_common] *ERROR* failed to set render ring head to zero ctl head tail start 3[ 3155.983364] [drm:stop_ring] *ERROR* bsd ring :timed out trying to stop ring 3[ 3156.986668] [drm:stop_ring] *ERROR* bsd ring :timed out trying to stop ring 3[ 3156.986673] [drm:init_ring_common] *ERROR* failed to set bsd ring head to zero ctl head tail start 3[ 3157.989982] [drm:stop_ring] *ERROR* render ring :timed out
Re: [Intel-gfx] [RFC] drm/i915/bdw: Apply workarounds to the golden render state
On 08/08/2014 10:57, Chris Wilson wrote: On Fri, Aug 08, 2014 at 10:52:57AM +0100, arun.siluv...@linux.intel.com wrote: From: Arun Siluvery arun.siluv...@linux.intel.com Workarounds for bdw are currently applied in init_clock_gating() but they are lost following a gpu reset. Some of the registers are part of register state context and they are restored with every context switch so initializing WAs in golden render state ensures that they are applied even when we start with an uninitialized context or during hw initialization followed by a reset. Interesting, but let's try to keep the opaque blobs minimal. The comments for w/a are even more valuable than the code. I agree, I will add comments to each workaround. We are looking at augmenting workarounds to the null batch in render state setup function itself. Do you have any comments with that approach? regards Arun -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Don't grab an fb reference for the idr
Hi
On Wed, Aug 6, 2014 at 9:10 AM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
The current refcounting scheme is that the fb lookup idr also holds a
reference. This works out nicely bacause thus far we've always
explicitly cleaned up idr entries for framebuffers:
- Userspace fbs get removed in the rmfb ioctl or when the drm file
gets closed.
- Kernel fbs (for fbdev emulation) get cleaned up by the driver code
at module unload time.
But now i915 also reconstructs the bios fbs for a smooth transition.
And that fb is purely transitional and should get removed immmediately
once all crtcs stop using it. Of course if the i915 fbdev code decides
to reuse it as the main fbdev fb then it shouldn't be cleaned up, but
in that case the fbdev code will grab it's own reference.
The problem is now that we also want to register that takeover fb in
the idr, so that userspace can do a smooth transition (animated maybe
even!) itself. But currently we have no one who will clean up the idr
reference once that fb isn't useful any more, and so essentially leak
it.
Fix this by no longer holding a full fb reference for the idr, but
instead just have a weak reference using kref_get_unless_zero. But
that requires us to synchronize and clean up with the idr and fb_lock
in drm_framebuffer_free, so add that. It's a bit ugly that we have to
unconditionally grab the fb_lock, but without that someone might creep
through a race.
This leak was caught by the fb leak check in drm_mode_config_cleanup.
Originally the leak was introduced in
commit 46f297fb83d4f9a6f6891964beb184664341a28b
Author: Jesse Barnes jbar...@virtuousgeek.org
Date: Fri Mar 7 08:57:48 2014 -0800
drm/i915: add plane_config fetching infrastructure v2
Cc: Jesse Barnes jbar...@virtuousgeek.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=77511
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/drm_crtc.c | 46
--
1 file changed, 28 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index fa2be24c..33ff631c8d23 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -515,9 +515,6 @@ int drm_framebuffer_init(struct drm_device *dev, struct
drm_framebuffer *fb,
if (ret)
goto out;
- /* Grab the idr reference. */
- drm_framebuffer_reference(fb);
-
dev-mode_config.num_fb++;
list_add(fb-head, dev-mode_config.fb_list);
out:
@@ -527,10 +524,34 @@ out:
}
EXPORT_SYMBOL(drm_framebuffer_init);
+/* dev-mode_config.fb_lock must be held! */
+static void __drm_framebuffer_unregister(struct drm_device *dev,
+struct drm_framebuffer *fb)
+{
+ mutex_lock(dev-mode_config.idr_mutex);
+ idr_remove(dev-mode_config.crtc_idr, fb-base.id);
+ mutex_unlock(dev-mode_config.idr_mutex);
+
+ fb-base.id = 0;
+}
+
static void drm_framebuffer_free(struct kref *kref)
{
struct drm_framebuffer *fb =
container_of(kref, struct drm_framebuffer, refcount);
+ struct drm_device *dev = fb-dev;
+
+ /*
+* The lookup idr holds a weak reference, which has not necessarily
been
+* removed at this point. Check for that.
+*/
+ mutex_lock(dev-mode_config.fb_lock);
+ if (fb-base.id) {
+ /* Mark fb as reaped and drop idr ref. */
+ __drm_framebuffer_unregister(dev, fb);
+ }
+ mutex_unlock(dev-mode_config.fb_lock);
Ewww, this is ugly. You now make the unregistration dynamic and it's
no longer under driver control. The typical device-control flow
assumes there's an authority that controls at which point objects are
registered and unregistered. You now bind it to ref-counts. To be
fair, I think lastclose is equally hackish (and doesn't really work
like you argued already).
I think the real problem is that you want two ref-counts: One
ref-count controls the object availability, the other ref-count
controls the visibility to user-space. This is also what gem does: you
have a kernel-internal ref-count for each gem-object, but you also
have user-space handles. A handle implies a kernel-internal ref-count
but not vice versa. And the object is only accessible from user-space,
as long as you own a handle. I think we want something similar for
FBs. This way you could unregister the bios-FB once no handle exists
(assuming a CRTC owns a handle as long as the FB is used as scan-out).
But I guess no-one wants to implement this, so I still think this
patch is the nicest way to make it work. So I'm fine with it.
Thanks
David
+
fb-funcs-destroy(fb);
}
@@ -567,8 +588,10 @@ struct drm_framebuffer *drm_framebuffer_lookup(struct
drm_device *dev,
mutex_lock(dev-mode_config.fb_lock);
fb = __drm_framebuffer_lookup(dev, id);
-
Re: [Intel-gfx] [RFC] drm/i915/bdw: Apply workarounds to the golden render state
On Fri, Aug 08, 2014 at 11:34:07AM +0100, Siluvery, Arun wrote: On 08/08/2014 10:57, Chris Wilson wrote: On Fri, Aug 08, 2014 at 10:52:57AM +0100, arun.siluv...@linux.intel.com wrote: From: Arun Siluvery arun.siluv...@linux.intel.com Workarounds for bdw are currently applied in init_clock_gating() but they are lost following a gpu reset. Some of the registers are part of register state context and they are restored with every context switch so initializing WAs in golden render state ensures that they are applied even when we start with an uninitialized context or during hw initialization followed by a reset. Interesting, but let's try to keep the opaque blobs minimal. The comments for w/a are even more valuable than the code. I agree, I will add comments to each workaround. We are looking at augmenting workarounds to the null batch in render state setup function itself. Do you have any comments with that approach? Other than changing its name, that does actually seem like a useful juncture to do things since it is the first userspace operation on the RCS ring within that context. And if it has to be within the context, that is indeed more troublesome to do without adding to the renderstate. -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 6/7] drm/tegra: Renaming DP training vswing pre emph defines
From: Sonika Jindal sonika.jin...@intel.com Rename the defines to have levels instead of values for vswing and pre-emph levels as the values may differ in other scenarios like low vswing of eDP1.4 where the values are different. Done using following cocci patch for each define: @@ @@ # define DP_TRAIN_VOLTAGE_SWING_400 (0 0) + # define DP_TRAIN_VOLTAGE_SWING_LEVEL_0 (0 0) ... Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/tegra/dpaux.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c index 708f783..d6b55e3 100644 --- a/drivers/gpu/drm/tegra/dpaux.c +++ b/drivers/gpu/drm/tegra/dpaux.c @@ -533,9 +533,9 @@ int tegra_dpaux_train(struct tegra_dpaux *dpaux, struct drm_dp_link *link, for (i = 0; i link-num_lanes; i++) values[i] = DP_TRAIN_MAX_PRE_EMPHASIS_REACHED | - DP_TRAIN_PRE_EMPHASIS_0 | + DP_TRAIN_PRE_EMPH_LEVEL_0 | DP_TRAIN_MAX_SWING_REACHED | - DP_TRAIN_VOLTAGE_SWING_400; + DP_TRAIN_VOLTAGE_SWING_LEVEL_0; err = drm_dp_dpcd_write(dpaux-aux, DP_TRAINING_LANE0_SET, values, link-num_lanes); -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/7] drm: Renaming DP training vswing pre emph defines
From: Sonika Jindal sonika.jin...@intel.com Adding new defines, older one will be removed in the last patch in the series. This is to rename the defines to have levels instead of values for vswing and pre-emph levels as the values may differ in other scenarios like low vswing of eDP1.4 where the values are different. Done using following cocci patch for each define: @@ @@ # define DP_TRAIN_VOLTAGE_SWING_400 (0 0) + # define DP_TRAIN_VOLTAGE_SWING_LEVEL_0 (0 0) ... Cc: dri-de...@lists.freedesktop.org Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- include/drm/drm_dp_helper.h |8 1 file changed, 8 insertions(+) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index a21568b..3840a05 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -191,15 +191,23 @@ # define DP_TRAIN_VOLTAGE_SWING_SHIFT 0 # define DP_TRAIN_MAX_SWING_REACHED(1 2) # define DP_TRAIN_VOLTAGE_SWING_400(0 0) +# define DP_TRAIN_VOLTAGE_SWING_LEVEL_0 (0 0) # define DP_TRAIN_VOLTAGE_SWING_600(1 0) +# define DP_TRAIN_VOLTAGE_SWING_LEVEL_1 (1 0) # define DP_TRAIN_VOLTAGE_SWING_800(2 0) +# define DP_TRAIN_VOLTAGE_SWING_LEVEL_2 (2 0) # define DP_TRAIN_VOLTAGE_SWING_1200 (3 0) +# define DP_TRAIN_VOLTAGE_SWING_LEVEL_3 (3 0) # define DP_TRAIN_PRE_EMPHASIS_MASK(3 3) # define DP_TRAIN_PRE_EMPHASIS_0 (0 3) +# define DP_TRAIN_PRE_EMPH_LEVEL_0 (0 3) # define DP_TRAIN_PRE_EMPHASIS_3_5 (1 3) +# define DP_TRAIN_PRE_EMPH_LEVEL_1 (1 3) # define DP_TRAIN_PRE_EMPHASIS_6 (2 3) +# define DP_TRAIN_PRE_EMPH_LEVEL_2 (2 3) # define DP_TRAIN_PRE_EMPHASIS_9_5 (3 3) +# define DP_TRAIN_PRE_EMPH_LEVEL_3 (3 3) # define DP_TRAIN_PRE_EMPHASIS_SHIFT 3 # define DP_TRAIN_MAX_PRE_EMPHASIS_REACHED (1 5) -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 0/7] Rename DP training vswing/pre-emph defines
From: Sonika Jindal sonika.jin...@intel.com Rename the defines to have levels instead of values for vswing and pre-emph levels as the values may differ in other scenarios like low vswing of eDP 1.4 where the values are different. Updated in all the drivers as well v2: Keeping the old defines in first patch and removing them in last patch. Used cocci semantic patch to replace the defines. Sonika Jindal (7): drm: Renaming DP training vswing pre emph defines drm/i915: Renaming DP training vswing pre emph defines drm/exynos: Renaming DP training vswing pre emph defines drm/gma500: Renaming DP training vswing pre emph defines drm/radeon: Renaming DP training vswing pre emph defines drm/tegra: Renaming DP training vswing pre emph defines drm: Remove old defines for vswing and pre-emph values drivers/gpu/drm/exynos/exynos_dp_core.c |4 +- drivers/gpu/drm/gma500/cdv_intel_dp.c |4 +- drivers/gpu/drm/gma500/intel_bios.c | 16 +-- drivers/gpu/drm/i915/intel_bios.c | 16 +-- drivers/gpu/drm/i915/intel_dp.c | 194 +++ drivers/gpu/drm/radeon/atombios_dp.c|4 +- drivers/gpu/drm/tegra/dpaux.c |4 +- include/drm/drm_dp_helper.h | 16 +-- 8 files changed, 129 insertions(+), 129 deletions(-) -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/7] drm/gma500: Renaming DP training vswing pre emph defines
From: Sonika Jindal sonika.jin...@intel.com
Rename the defines to have levels instead of values for vswing and
pre-emph levels as the values may differ in other scenarios like low vswing of
eDP1.4 where the values are different.
Done using following cocci patch for each define:
@@
@@
# define DP_TRAIN_VOLTAGE_SWING_400 (0 0)
+ # define DP_TRAIN_VOLTAGE_SWING_LEVEL_0 (0 0)
...
Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
drivers/gpu/drm/gma500/cdv_intel_dp.c |4 ++--
drivers/gpu/drm/gma500/intel_bios.c | 16
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c
b/drivers/gpu/drm/gma500/cdv_intel_dp.c
index a4cc0e6..9f158ea 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_dp.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c
@@ -1089,7 +1089,7 @@ static char *link_train_names[] = {
};
#endif
-#define CDV_DP_VOLTAGE_MAX DP_TRAIN_VOLTAGE_SWING_1200
+#define CDV_DP_VOLTAGE_MAX DP_TRAIN_VOLTAGE_SWING_LEVEL_3
/*
static uint8_t
cdv_intel_dp_pre_emphasis_max(uint8_t voltage_swing)
@@ -1276,7 +1276,7 @@ cdv_intel_dp_set_vswing_premph(struct gma_encoder
*encoder, uint8_t signal_level
cdv_sb_write(dev, ddi_reg-VSwing2,
dp_vswing_premph_table[index]);
/* ;gfx_dpio_set_reg(0x814c, 0x40802040) */
- if ((vswing + premph) == DP_TRAIN_VOLTAGE_SWING_1200)
+ if ((vswing + premph) == DP_TRAIN_VOLTAGE_SWING_LEVEL_3)
cdv_sb_write(dev, ddi_reg-VSwing3, 0x70802040);
else
cdv_sb_write(dev, ddi_reg-VSwing3, 0x40802040);
diff --git a/drivers/gpu/drm/gma500/intel_bios.c
b/drivers/gpu/drm/gma500/intel_bios.c
index d349734..63bde4e 100644
--- a/drivers/gpu/drm/gma500/intel_bios.c
+++ b/drivers/gpu/drm/gma500/intel_bios.c
@@ -116,30 +116,30 @@ parse_edp(struct drm_psb_private *dev_priv, struct
bdb_header *bdb)
switch (edp_link_params-preemphasis) {
case 0:
- dev_priv-edp.preemphasis = DP_TRAIN_PRE_EMPHASIS_0;
+ dev_priv-edp.preemphasis = DP_TRAIN_PRE_EMPH_LEVEL_0;
break;
case 1:
- dev_priv-edp.preemphasis = DP_TRAIN_PRE_EMPHASIS_3_5;
+ dev_priv-edp.preemphasis = DP_TRAIN_PRE_EMPH_LEVEL_1;
break;
case 2:
- dev_priv-edp.preemphasis = DP_TRAIN_PRE_EMPHASIS_6;
+ dev_priv-edp.preemphasis = DP_TRAIN_PRE_EMPH_LEVEL_2;
break;
case 3:
- dev_priv-edp.preemphasis = DP_TRAIN_PRE_EMPHASIS_9_5;
+ dev_priv-edp.preemphasis = DP_TRAIN_PRE_EMPH_LEVEL_3;
break;
}
switch (edp_link_params-vswing) {
case 0:
- dev_priv-edp.vswing = DP_TRAIN_VOLTAGE_SWING_400;
+ dev_priv-edp.vswing = DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
break;
case 1:
- dev_priv-edp.vswing = DP_TRAIN_VOLTAGE_SWING_600;
+ dev_priv-edp.vswing = DP_TRAIN_VOLTAGE_SWING_LEVEL_1;
break;
case 2:
- dev_priv-edp.vswing = DP_TRAIN_VOLTAGE_SWING_800;
+ dev_priv-edp.vswing = DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
break;
case 3:
- dev_priv-edp.vswing = DP_TRAIN_VOLTAGE_SWING_1200;
+ dev_priv-edp.vswing = DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
break;
}
DRM_DEBUG_KMS(VBT reports EDP: VSwing %d, Preemph %d\n,
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/7] drm/radeon: Renaming DP training vswing pre emph defines
From: Sonika Jindal sonika.jin...@intel.com Rename the defines to have levels instead of values for vswing and pre-emph levels as the values may differ in other scenarios like low vswing of eDP1.4 where the values are different. Done using following cocci patch for each define: @@ @@ # define DP_TRAIN_VOLTAGE_SWING_1200 (3 0) + # define DP_TRAIN_VOLTAGE_SWING_LEVEL_3 (0 0) ... Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/radeon/atombios_dp.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c index b1e11f8..95ea276 100644 --- a/drivers/gpu/drm/radeon/atombios_dp.c +++ b/drivers/gpu/drm/radeon/atombios_dp.c @@ -232,8 +232,8 @@ void radeon_dp_aux_init(struct radeon_connector *radeon_connector) /* general DP utility functions */ -#define DP_VOLTAGE_MAX DP_TRAIN_VOLTAGE_SWING_1200 -#define DP_PRE_EMPHASIS_MAXDP_TRAIN_PRE_EMPHASIS_9_5 +#define DP_VOLTAGE_MAX DP_TRAIN_VOLTAGE_SWING_LEVEL_3 +#define DP_PRE_EMPHASIS_MAXDP_TRAIN_PRE_EMPH_LEVEL_3 static void dp_get_adjust_train(u8 link_status[DP_LINK_STATUS_SIZE], int lane_count, -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 7/7] drm: Remove old defines for vswing and pre-emph values
From: Sonika Jindal sonika.jin...@intel.com This is the last patch in the series, so remove old defines Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- include/drm/drm_dp_helper.h |8 1 file changed, 8 deletions(-) diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 3840a05..9305c71 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -190,23 +190,15 @@ # define DP_TRAIN_VOLTAGE_SWING_MASK 0x3 # define DP_TRAIN_VOLTAGE_SWING_SHIFT 0 # define DP_TRAIN_MAX_SWING_REACHED(1 2) -# define DP_TRAIN_VOLTAGE_SWING_400(0 0) # define DP_TRAIN_VOLTAGE_SWING_LEVEL_0 (0 0) -# define DP_TRAIN_VOLTAGE_SWING_600(1 0) # define DP_TRAIN_VOLTAGE_SWING_LEVEL_1 (1 0) -# define DP_TRAIN_VOLTAGE_SWING_800(2 0) # define DP_TRAIN_VOLTAGE_SWING_LEVEL_2 (2 0) -# define DP_TRAIN_VOLTAGE_SWING_1200 (3 0) # define DP_TRAIN_VOLTAGE_SWING_LEVEL_3 (3 0) # define DP_TRAIN_PRE_EMPHASIS_MASK(3 3) -# define DP_TRAIN_PRE_EMPHASIS_0 (0 3) # define DP_TRAIN_PRE_EMPH_LEVEL_0 (0 3) -# define DP_TRAIN_PRE_EMPHASIS_3_5 (1 3) # define DP_TRAIN_PRE_EMPH_LEVEL_1 (1 3) -# define DP_TRAIN_PRE_EMPHASIS_6 (2 3) # define DP_TRAIN_PRE_EMPH_LEVEL_2 (2 3) -# define DP_TRAIN_PRE_EMPHASIS_9_5 (3 3) # define DP_TRAIN_PRE_EMPH_LEVEL_3 (3 3) # define DP_TRAIN_PRE_EMPHASIS_SHIFT 3 -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/7] drm/i915: Renaming DP training vswing pre emph defines
From: Sonika Jindal sonika.jin...@intel.com
Rename the defines to have levels instead of values for vswing and
pre-emph levels as the values may differ in other scenarios like low vswing of
eDP1.4 where the values are different.
Done using following cocci patch for each define:
@@
@@
# define DP_TRAIN_VOLTAGE_SWING_400 (0 0)
+ # define DP_TRAIN_VOLTAGE_SWING_LEVEL_0 (0 0)
...
Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
drivers/gpu/drm/i915/intel_bios.c | 16 +--
drivers/gpu/drm/i915/intel_dp.c | 194 ++---
2 files changed, 105 insertions(+), 105 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.c
b/drivers/gpu/drm/i915/intel_bios.c
index 031c565..e871f68 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -627,16 +627,16 @@ parse_edp(struct drm_i915_private *dev_priv, struct
bdb_header *bdb)
switch (edp_link_params-preemphasis) {
case EDP_PREEMPHASIS_NONE:
- dev_priv-vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_0;
+ dev_priv-vbt.edp_preemphasis = DP_TRAIN_PRE_EMPH_LEVEL_0;
break;
case EDP_PREEMPHASIS_3_5dB:
- dev_priv-vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_3_5;
+ dev_priv-vbt.edp_preemphasis = DP_TRAIN_PRE_EMPH_LEVEL_1;
break;
case EDP_PREEMPHASIS_6dB:
- dev_priv-vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_6;
+ dev_priv-vbt.edp_preemphasis = DP_TRAIN_PRE_EMPH_LEVEL_2;
break;
case EDP_PREEMPHASIS_9_5dB:
- dev_priv-vbt.edp_preemphasis = DP_TRAIN_PRE_EMPHASIS_9_5;
+ dev_priv-vbt.edp_preemphasis = DP_TRAIN_PRE_EMPH_LEVEL_3;
break;
default:
DRM_DEBUG_KMS(VBT has unknown eDP pre-emphasis value %u\n,
@@ -646,16 +646,16 @@ parse_edp(struct drm_i915_private *dev_priv, struct
bdb_header *bdb)
switch (edp_link_params-vswing) {
case EDP_VSWING_0_4V:
- dev_priv-vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_400;
+ dev_priv-vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
break;
case EDP_VSWING_0_6V:
- dev_priv-vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_600;
+ dev_priv-vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_LEVEL_1;
break;
case EDP_VSWING_0_8V:
- dev_priv-vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_800;
+ dev_priv-vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
break;
case EDP_VSWING_1_2V:
- dev_priv-vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_1200;
+ dev_priv-vbt.edp_vswing = DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
break;
default:
DRM_DEBUG_KMS(VBT has unknown eDP voltage swing value %u\n,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 34e3c47..01f264c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2381,13 +2381,13 @@ intel_dp_voltage_max(struct intel_dp *intel_dp)
enum port port = dp_to_dig_port(intel_dp)-port;
if (IS_VALLEYVIEW(dev))
- return DP_TRAIN_VOLTAGE_SWING_1200;
+ return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
else if (IS_GEN7(dev) port == PORT_A)
- return DP_TRAIN_VOLTAGE_SWING_800;
+ return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
else if (HAS_PCH_CPT(dev) port != PORT_A)
- return DP_TRAIN_VOLTAGE_SWING_1200;
+ return DP_TRAIN_VOLTAGE_SWING_LEVEL_3;
else
- return DP_TRAIN_VOLTAGE_SWING_800;
+ return DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
}
static uint8_t
@@ -2398,49 +2398,49 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp,
uint8_t voltage_swing)
if (IS_HASWELL(dev) || IS_BROADWELL(dev)) {
switch (voltage_swing DP_TRAIN_VOLTAGE_SWING_MASK) {
- case DP_TRAIN_VOLTAGE_SWING_400:
- return DP_TRAIN_PRE_EMPHASIS_9_5;
- case DP_TRAIN_VOLTAGE_SWING_600:
- return DP_TRAIN_PRE_EMPHASIS_6;
- case DP_TRAIN_VOLTAGE_SWING_800:
- return DP_TRAIN_PRE_EMPHASIS_3_5;
- case DP_TRAIN_VOLTAGE_SWING_1200:
+ case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
+ return DP_TRAIN_PRE_EMPH_LEVEL_3;
+ case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
+ return DP_TRAIN_PRE_EMPH_LEVEL_2;
+ case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
+ return DP_TRAIN_PRE_EMPH_LEVEL_1;
+ case DP_TRAIN_VOLTAGE_SWING_LEVEL_3:
default:
- return DP_TRAIN_PRE_EMPHASIS_0;
+ return DP_TRAIN_PRE_EMPH_LEVEL_0;
}
} else if (IS_VALLEYVIEW(dev)) {
[Intel-gfx] [PATCH 3/7] drm/exynos: Renaming DP training vswing pre emph defines
From: Sonika Jindal sonika.jin...@intel.com Rename the defines to have levels instead of values for vswing and pre-emph levels as the values may differ in other scenarios like low vswing of eDP1.4 where the values are different. Done using following cocci patch for each define: @@ @@ # define DP_TRAIN_VOLTAGE_SWING_400 (0 0) + # define DP_TRAIN_VOLTAGE_SWING_LEVEL_0 (0 0) ... Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/exynos/exynos_dp_core.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index 4f3c7eb..02602a8 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -329,8 +329,8 @@ static int exynos_dp_link_start(struct exynos_dp_device *dp) return retval; for (lane = 0; lane lane_count; lane++) - buf[lane] = DP_TRAIN_PRE_EMPHASIS_0 | - DP_TRAIN_VOLTAGE_SWING_400; + buf[lane] = DP_TRAIN_PRE_EMPH_LEVEL_0 | + DP_TRAIN_VOLTAGE_SWING_LEVEL_0; retval = exynos_dp_write_bytes_to_dpcd(dp, DP_TRAINING_LANE0_SET, lane_count, buf); -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.
On Fri, 2014-08-08 at 15:54 +0530, Sagar Arun Kamble wrote: On Fri, 2014-08-08 at 11:15 +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 02:29:38PM +0530, Sagar Arun Kamble wrote: On Fri, 2014-08-08 at 09:42 +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 12:22:44PM +0530, Sagar Arun Kamble wrote: Hi Daniel, On Mon, 2014-08-04 at 10:07 +0200, Daniel Vetter wrote: On Fri, Aug 01, 2014 at 12:34:56PM +0530, sagar.a.kam...@intel.com wrote: From: Sagar Kamble sagar.a.kam...@intel.com @@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev) intel_display_set_init_power(dev_priv, false); - return 0; + /* Save Gunit State and clear wake - Need to make sure + * changes in vlv_runtime_suspend path don't impact this path */ + if (IS_VALLEYVIEW(dev)) + ret = vlv_runtime_suspend(dev_priv); Maybe I wasn't clear, but I absolutely don't want any IS_VLV additions to core resume/thaw code. This should be shovelled into the runtime pm handling code, which should be reused in the suspend/resume code. This piece of code does not fit into any of the power well get/put path. Its specific sequence that need to be followed in VLV when Gunit gets power gated. So we have to keep this IS_VLV related functionality in both runtime and pm suspend/resume. Well we support S0ix now. Which means our system suspend/resume code actually calls into the runtime pm code. So either that design is broken (and we need to fix this) or something else is amiss. Or we don't need this code any more. But duplicating it is not the right approach. And yeah the power domain stuff might not be the right place, I've just used that as a place-holder for all the runtime pm code we have. -Daniel While entering S0iX we are getting following call flow: intel_runtime_resume followed by i915_drm_freeze and While resuming back from S0iX: i915_drm_thaw How can we share that wake control, gunit save/restore functionality? I am observing issue given below: Here is how I am achieving S0ix: 1. echo mem /sys/power/state 2. (Assuming we have removed all wake_locks in /sys/powe/wake_lock) If device is runtime suspend it is getting resumed with runtime_resume 3. PM core triggers this sequence for each device (prepare, suspend, suspend_late, suspend_noirq) 4. Then pm_suspend happens for Gfx. After all devices are suspended S0i0-S0iR-S0i1-S0i2-S0i3 happens. Gunit gets power gated. While resuming back (with user action/power button press) 1. PM core triggers following sequenc for each device: (resume_noirq, resume_early, resume, complete) After call to i915_drm_thaw_early(called through resume_early), during i915_drm_thaw(called through resume) we were seeing following issue during ring initialization. This is happening since wake is not enabled and Gunit registers are not restored(which is done in runtime_resume but that path is not taken here since this is resume from pm_suspend) We have intel_runtime_pm_get/put calls in the suspend/resume paths which should achieve this. Maybe they're not at the right place or not in the right ordering, but they're there. So on platforms with runtime pm support we _do_ call all the relevant runtime pm callbacks from system suspend/resume. -Daniel I am seeing following commit removed get/put calls from suspend resume paths: commit 395a5abbd97d7d76a7a26da52f33daebe279b3b3 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Wed Jun 18 09:52:56 2014 -0700 drm/i915: don't take runtime PM reference around freeze/thaw That ordering of get/put was also incorrect probably. Even though we do rpm_get before freeze after S0i3 we need to explicitely do wake contro/gunit save restore. In the current patch, the rpm handlers are called directly instead of (intel_runtime_pm_get/put). valleyview_runtime_suspend at the end of i915_drm_freeze and valleyview_runtime_resume at the beginning of i915_drm_thaw_early. Do I need to replace them with intel_display_power_get/put? Will DPM core allow rpm calls when system pm suspend/resume is in progress? No, you can't make the device runtime suspend while the system suspend handler runs since DPM takes an RPM reference before calling this handler. The reference will be dropped only after returning from the corresponding system resume handler. 6[ 3152.991399] PM: device[controlD64] driver[drm] resume enter 3[ 3153.976759] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring 3[ 3154.980061] [drm:stop_ring] *ERROR* render ring :timed out trying to stop ring 3[ 3154.980067] [drm:init_ring_common] *ERROR* failed to set render ring head to zero ctl head tail
[Intel-gfx] [PATCH] drm/i915: Continuation of future readiness series
From: Sonika Jindal sonika.jin...@intel.com
Removing the check for HAS_PCH_SPLIT, it looks redundant here. Anyways all the
platforms are checked separately.
Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 40 --
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 89e0ac5..5a3e239 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12421,27 +12421,25 @@ static void intel_init_display(struct drm_device *dev)
dev_priv-display.get_display_clock_speed =
i830_get_display_clock_speed;
- if (HAS_PCH_SPLIT(dev)) {
- if (IS_GEN5(dev)) {
- dev_priv-display.fdi_link_train =
ironlake_fdi_link_train;
- dev_priv-display.write_eld = ironlake_write_eld;
- } else if (IS_GEN6(dev)) {
- dev_priv-display.fdi_link_train = gen6_fdi_link_train;
- dev_priv-display.write_eld = ironlake_write_eld;
- dev_priv-display.modeset_global_resources =
- snb_modeset_global_resources;
- } else if (IS_IVYBRIDGE(dev)) {
- /* FIXME: detect B0+ stepping and use auto training */
- dev_priv-display.fdi_link_train =
ivb_manual_fdi_link_train;
- dev_priv-display.write_eld = ironlake_write_eld;
- dev_priv-display.modeset_global_resources =
- ivb_modeset_global_resources;
- } else if (IS_HASWELL(dev) || IS_GEN8(dev)) {
- dev_priv-display.fdi_link_train = hsw_fdi_link_train;
- dev_priv-display.write_eld = haswell_write_eld;
- dev_priv-display.modeset_global_resources =
- haswell_modeset_global_resources;
- }
+ if (IS_GEN5(dev)) {
+ dev_priv-display.fdi_link_train = ironlake_fdi_link_train;
+ dev_priv-display.write_eld = ironlake_write_eld;
+ } else if (IS_GEN6(dev)) {
+ dev_priv-display.fdi_link_train = gen6_fdi_link_train;
+ dev_priv-display.write_eld = ironlake_write_eld;
+ dev_priv-display.modeset_global_resources =
+ snb_modeset_global_resources;
+ } else if (IS_IVYBRIDGE(dev)) {
+ /* FIXME: detect B0+ stepping and use auto training */
+ dev_priv-display.fdi_link_train = ivb_manual_fdi_link_train;
+ dev_priv-display.write_eld = ironlake_write_eld;
+ dev_priv-display.modeset_global_resources =
+ ivb_modeset_global_resources;
+ } else if (IS_HASWELL(dev) || IS_GEN8(dev)) {
+ dev_priv-display.fdi_link_train = hsw_fdi_link_train;
+ dev_priv-display.write_eld = haswell_write_eld;
+ dev_priv-display.modeset_global_resources =
+ haswell_modeset_global_resources;
} else if (IS_G4X(dev)) {
dev_priv-display.write_eld = g4x_write_eld;
} else if (IS_VALLEYVIEW(dev)) {
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Continuation of future readiness series
On Fri, Aug 08, 2014 at 05:09:14PM +0530, sonika.jin...@intel.com wrote:
From: Sonika Jindal sonika.jin...@intel.com
Removing the check for HAS_PCH_SPLIT, it looks redundant here. Anyways all the
platforms are checked separately.
Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 40
--
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 89e0ac5..5a3e239 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12421,27 +12421,25 @@ static void intel_init_display(struct drm_device
*dev)
dev_priv-display.get_display_clock_speed =
i830_get_display_clock_speed;
- if (HAS_PCH_SPLIT(dev)) {
- if (IS_GEN5(dev)) {
- dev_priv-display.fdi_link_train =
ironlake_fdi_link_train;
- dev_priv-display.write_eld = ironlake_write_eld;
- } else if (IS_GEN6(dev)) {
- dev_priv-display.fdi_link_train = gen6_fdi_link_train;
- dev_priv-display.write_eld = ironlake_write_eld;
- dev_priv-display.modeset_global_resources =
- snb_modeset_global_resources;
- } else if (IS_IVYBRIDGE(dev)) {
- /* FIXME: detect B0+ stepping and use auto training */
- dev_priv-display.fdi_link_train =
ivb_manual_fdi_link_train;
- dev_priv-display.write_eld = ironlake_write_eld;
- dev_priv-display.modeset_global_resources =
- ivb_modeset_global_resources;
- } else if (IS_HASWELL(dev) || IS_GEN8(dev)) {
- dev_priv-display.fdi_link_train = hsw_fdi_link_train;
- dev_priv-display.write_eld = haswell_write_eld;
- dev_priv-display.modeset_global_resources =
- haswell_modeset_global_resources;
- }
+ if (IS_GEN5(dev)) {
+ dev_priv-display.fdi_link_train = ironlake_fdi_link_train;
+ dev_priv-display.write_eld = ironlake_write_eld;
+ } else if (IS_GEN6(dev)) {
+ dev_priv-display.fdi_link_train = gen6_fdi_link_train;
+ dev_priv-display.write_eld = ironlake_write_eld;
+ dev_priv-display.modeset_global_resources =
+ snb_modeset_global_resources;
+ } else if (IS_IVYBRIDGE(dev)) {
+ /* FIXME: detect B0+ stepping and use auto training */
+ dev_priv-display.fdi_link_train = ivb_manual_fdi_link_train;
+ dev_priv-display.write_eld = ironlake_write_eld;
+ dev_priv-display.modeset_global_resources =
+ ivb_modeset_global_resources;
+ } else if (IS_HASWELL(dev) || IS_GEN8(dev)) {
+ dev_priv-display.fdi_link_train = hsw_fdi_link_train;
+ dev_priv-display.write_eld = haswell_write_eld;
+ dev_priv-display.modeset_global_resources =
+ haswell_modeset_global_resources;
Maybe shuffle these around a bit while you're at it so that the checks
would be roughly in gen order.
} else if (IS_G4X(dev)) {
dev_priv-display.write_eld = g4x_write_eld;
} else if (IS_VALLEYVIEW(dev)) {
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] drm/i915/bdw: Apply workarounds to the golden render state
On Fri, Aug 08, 2014 at 10:52:57AM +0100, arun.siluv...@linux.intel.com wrote: From: Arun Siluvery arun.siluv...@linux.intel.com Workarounds for bdw are currently applied in init_clock_gating() but they are lost following a gpu reset. Some of the registers are part of register state context and they are restored with every context switch so initializing WAs in golden render state ensures that they are applied even when we start with an uninitialized context or during hw initialization followed by a reset. This approach might require separate null states for BDW vs. CHV and IVB vs. HSW vs. VLV, which seems a bit unfortunate. Might be better to just issue the w/a register writes via LRIs from the code as part of the null state load. Although I don't actually undertand how this improves things as opposed to just appllying the w/as via mmio writes. Does it? Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/intel_pm.c | 50 - drivers/gpu/drm/i915/intel_renderstate_gen8.c | 62 +-- 2 files changed, 39 insertions(+), 73 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 1ddd4df..ab64b64 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5402,38 +5402,11 @@ static void gen8_init_clock_gating(struct drm_device *dev) /* FIXME(BDW): Check all the w/a, some might only apply to * pre-production hw. */ - /* WaDisablePartialInstShootdown:bdw */ - I915_WRITE(GEN8_ROW_CHICKEN, -_MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE)); - - /* WaDisableThreadStallDopClockGating:bdw */ - /* FIXME: Unclear whether we really need this on production bdw. */ - I915_WRITE(GEN8_ROW_CHICKEN, -_MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE)); - - /* - * This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for - * pre-production hardware - */ - I915_WRITE(HALF_SLICE_CHICKEN3, -_MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS)); - I915_WRITE(HALF_SLICE_CHICKEN3, -_MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS)); I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_BWGTLB_DISABLE)); I915_WRITE(_3D_CHICKEN3, _MASKED_BIT_ENABLE(_3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(2))); - I915_WRITE(COMMON_SLICE_CHICKEN2, -_MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE)); - - I915_WRITE(GEN7_HALF_SLICE_CHICKEN1, -_MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE)); - - /* WaDisableDopClockGating:bdw May not be needed for production */ - I915_WRITE(GEN7_ROW_CHICKEN2, -_MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); - /* WaSwitchSolVfFArbitrationPriority:bdw */ I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL); @@ -5448,41 +5421,18 @@ static void gen8_init_clock_gating(struct drm_device *dev) BDW_DPRS_MASK_VBLANK_SRD); } - /* Use Force Non-Coherent whenever executing a 3D context. This is a - * workaround for for a possible hang in the unlikely event a TLB - * invalidation occurs during a PSD flush. - */ - I915_WRITE(HDC_CHICKEN0, -I915_READ(HDC_CHICKEN0) | -_MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT)); - /* WaVSRefCountFullforceMissDisable:bdw */ /* WaDSRefCountFullforceMissDisable:bdw */ I915_WRITE(GEN7_FF_THREAD_MODE, I915_READ(GEN7_FF_THREAD_MODE) ~(GEN8_FF_DS_REF_CNT_FFME | GEN7_FF_VS_REF_CNT_FFME)); - /* - * BSpec recommends 8x4 when MSAA is used, - * however in practice 16x4 seems fastest. - * - * Note that PS/WM thread counts depend on the WIZ hashing - * disable bit, which we don't touch here, but it's good - * to keep in mind (see 3DSTATE_PS and 3DSTATE_WM). - */ - I915_WRITE(GEN7_GT_MODE, -GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4); - I915_WRITE(GEN6_RC_SLEEP_PSMI_CONTROL, _MASKED_BIT_ENABLE(GEN8_RC_SEMA_IDLE_MSG_DISABLE)); /* WaDisableSDEUnitClockGating:bdw */ I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | GEN8_SDEUNIT_CLOCK_GATE_DISABLE); - - /* Wa4x4STCOptimizationDisable:bdw */ - I915_WRITE(CACHE_MODE_1, -_MASKED_BIT_ENABLE(GEN8_4x4_STC_OPTIMIZATION_DISABLE)); } static void haswell_init_clock_gating(struct drm_device *dev) diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen8.c b/drivers/gpu/drm/i915/intel_renderstate_gen8.c index 75ef1b5..0b26783 100644 --- a/drivers/gpu/drm/i915/intel_renderstate_gen8.c +++ b/drivers/gpu/drm/i915/intel_renderstate_gen8.c @@ -1,14 +1,38 @@ #include
[Intel-gfx] [PATCH v2] drm/i915: Rename defines for selection of ddi buffer translation slot
From: Sonika Jindal sonika.jin...@intel.com
Renaming the HSW-specific macros for ddi buffer translation slot to denote the
slot and not the vswing/pre-emph values as they are platform-dependent.
This patch is based on top of the patch series for renaming the DP training
vswing/pre-emph defines:
http://lists.freedesktop.org/archives/intel-gfx/2014-August/050407.html
v2: Creating single macro with argument for slot number (Damien)
Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h | 10 +-
drivers/gpu/drm/i915/intel_ddi.c | 19 +++
drivers/gpu/drm/i915/intel_dp.c | 18 +-
3 files changed, 13 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7a6cc69..7366aac 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5966,15 +5966,7 @@ enum punit_power_well {
#define DDI_BUF_CTL_B 0x64100
#define DDI_BUF_CTL(port) _PORT(port, DDI_BUF_CTL_A, DDI_BUF_CTL_B)
#define DDI_BUF_CTL_ENABLE(131)
-#define DDI_BUF_EMP_400MV_0DB_HSW (024) /* Sel0 */
-#define DDI_BUF_EMP_400MV_3_5DB_HSW (124) /* Sel1 */
-#define DDI_BUF_EMP_400MV_6DB_HSW (224) /* Sel2 */
-#define DDI_BUF_EMP_400MV_9_5DB_HSW (324) /* Sel3 */
-#define DDI_BUF_EMP_600MV_0DB_HSW (424) /* Sel4 */
-#define DDI_BUF_EMP_600MV_3_5DB_HSW (524) /* Sel5 */
-#define DDI_BUF_EMP_600MV_6DB_HSW (624) /* Sel6 */
-#define DDI_BUF_EMP_800MV_0DB_HSW (724) /* Sel7 */
-#define DDI_BUF_EMP_800MV_3_5DB_HSW (824) /* Sel8 */
+#define DDI_BUF_TRANS_SELECT(n) ((n) 24)
#define DDI_BUF_EMP_MASK (0xf24)
#define DDI_BUF_PORT_REVERSAL (116)
#define DDI_BUF_IS_IDLE (17)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ca1f9a8..493cf3a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -241,18 +241,6 @@ void intel_prepare_ddi(struct drm_device *dev)
intel_prepare_ddi_buffers(dev, port);
}
-static const long hsw_ddi_buf_ctl_values[] = {
- DDI_BUF_EMP_400MV_0DB_HSW,
- DDI_BUF_EMP_400MV_3_5DB_HSW,
- DDI_BUF_EMP_400MV_6DB_HSW,
- DDI_BUF_EMP_400MV_9_5DB_HSW,
- DDI_BUF_EMP_600MV_0DB_HSW,
- DDI_BUF_EMP_600MV_3_5DB_HSW,
- DDI_BUF_EMP_600MV_6DB_HSW,
- DDI_BUF_EMP_800MV_0DB_HSW,
- DDI_BUF_EMP_800MV_3_5DB_HSW
-};
-
static void intel_wait_ddi_buf_idle(struct drm_i915_private *dev_priv,
enum port port)
{
@@ -275,7 +263,6 @@ static void intel_wait_ddi_buf_idle(struct drm_i915_private
*dev_priv,
* please note that when FDI mode is active on DDI E, it shares 2 lines with
* DDI A (which is used for eDP)
*/
-
void hsw_fdi_link_train(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc-dev;
@@ -312,7 +299,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
/* Start the training iterating through available voltages and emphasis,
* testing each value twice. */
- for (i = 0; i ARRAY_SIZE(hsw_ddi_buf_ctl_values) * 2; i++) {
+ for (i = 0; i ARRAY_SIZE(hsw_ddi_translations_fdi); i++) {
/* Configure DP_TP_CTL with auto-training */
I915_WRITE(DP_TP_CTL(PORT_E),
DP_TP_CTL_FDI_AUTOTRAIN |
@@ -327,7 +314,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
I915_WRITE(DDI_BUF_CTL(PORT_E),
DDI_BUF_CTL_ENABLE |
((intel_crtc-config.fdi_lanes - 1) 1) |
- hsw_ddi_buf_ctl_values[i / 2]);
+ DDI_BUF_TRANS_SELECT(i / 2));
POSTING_READ(DDI_BUF_CTL(PORT_E));
udelay(600);
@@ -402,7 +389,7 @@ void intel_ddi_init_dp_buf_reg(struct intel_encoder
*encoder)
enc_to_dig_port(encoder-base);
intel_dp-DP = intel_dig_port-saved_port_bits |
- DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW;
+ DDI_BUF_CTL_ENABLE | DDI_BUF_TRANS_SELECT(0);
intel_dp-DP |= DDI_PORT_WIDTH(intel_dp-lane_count);
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 01f264c..7215cfe 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2842,25 +2842,25 @@ intel_hsw_signal_levels(uint8_t train_set)
DP_TRAIN_PRE_EMPHASIS_MASK);
switch (signal_levels) {
case DP_TRAIN_VOLTAGE_SWING_LEVEL_0 | DP_TRAIN_PRE_EMPH_LEVEL_0:
- return DDI_BUF_EMP_400MV_0DB_HSW;
+ return DDI_BUF_TRANS_SELECT(0);
case DP_TRAIN_VOLTAGE_SWING_LEVEL_0 | DP_TRAIN_PRE_EMPH_LEVEL_1:
-
Re: [Intel-gfx] [PATCH v2] drm/i915: Rename defines for selection of ddi buffer translation slot
On Fri, Aug 08, 2014 at 05:47:25PM +0530, sonika.jin...@intel.com wrote:
/* Start the training iterating through available voltages and emphasis,
* testing each value twice. */
- for (i = 0; i ARRAY_SIZE(hsw_ddi_buf_ctl_values) * 2; i++) {
+ for (i = 0; i ARRAY_SIZE(hsw_ddi_translations_fdi); i++) {
This is correct but obsfucated. You're actually doing
ARRAY_SIZE(hsw_ddi_translations_fdi) / 2 * 2 and we might as well give
the ARRAY_SIZE(hsw_ddi_translations_fdi) / 2 a proper name like I
suggested.
With that fixed, you can add my:
Reviewed-by: Damien Lespiau damien.lesp...@intel.com
--
Damien
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/15] drm/i915: Some cleanups for the ppgtt lifetime handling
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch]
Sent: Wednesday, August 06, 2014 2:05 PM
To: Intel Graphics Development
Cc: Daniel Vetter; Thierry, Michel; Chris Wilson
Subject: [PATCH 02/15] drm/i915: Some cleanups for the ppgtt lifetime
handling
So when reviewing Michel's patch I've noticed a few things and cleaned
them up:
- The early checks in ppgtt_release are now redundant: The inactive
list should always be empty now, so we can ditch these checks. Even
for the aliasing ppgtt (though that's a different confusion) since
we tear that down after all the objects are gone.
- The ppgtt handling functions are splattered all over. Consolidate
them in i915_gem_gtt.c, give them OCD prefixes and add wrappers for
get/put.
- There was a bit a confusion in ppgtt_release about whether it cares
about the active or inactive list. It should care about them both,
so augment the WARNINGs to check for both.
There's still create_vm_for_ctx left to do, put that is blocked on the
removal of ppgtt-ctx. Once that's done we can rename it to
i915_ppgtt_create and move it to its siblings for handling ppgtts.
v2: Move the ppgtt checks into the inline get/put functions as
suggested by Chris.
v3: Inline the now redundant ppgtt local variable.
Cc: Michel Thierry michel.thie...@intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/i915_gem.c | 5 +
drivers/gpu/drm/i915/i915_gem_context.c | 36
-
drivers/gpu/drm/i915/i915_gem_gtt.c | 20 +-
drivers/gpu/drm/i915/i915_gem_gtt.h | 14 -
5 files changed, 33 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 5a18680011da..d349dd75ed69 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2512,7 +2512,6 @@ void i915_gem_object_ggtt_unpin(struct
drm_i915_gem_object *obj);
#define ctx_to_ppgtt(ctx) container_of((ctx)-vm, struct i915_hw_ppgtt,
base)
#define vm_to_ppgtt(vm) container_of(vm, struct i915_hw_ppgtt, base)
int __must_check i915_gem_context_init(struct drm_device *dev);
-void ppgtt_release(struct kref *kref);
void i915_gem_context_fini(struct drm_device *dev);
void i915_gem_context_reset(struct drm_device *dev);
int i915_gem_context_open(struct drm_device *dev, struct drm_file *file);
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index 25a32b9c9b4b..b33a677b4b1e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4500,7 +4500,6 @@ struct i915_vma *i915_gem_obj_to_vma(struct
drm_i915_gem_object *obj,
void i915_gem_vma_destroy(struct i915_vma *vma)
{
struct i915_address_space *vm = NULL;
- struct i915_hw_ppgtt *ppgtt = NULL;
WARN_ON(vma-node.allocated);
/* Keep the vma as a placeholder in the execbuffer reservation lists
*/
@@ -4508,10 +4507,8 @@ void i915_gem_vma_destroy(struct i915_vma
*vma)
return;
vm = vma-vm;
- ppgtt = vm_to_ppgtt(vm);
- if (ppgtt)
- kref_put(ppgtt-ref, ppgtt_release);
+ i915_ppgtt_put(vm_to_ppgtt(vm));
list_del(vma-vma_link);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index ae706cba05ae..899c6a7a5920 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -96,33 +96,6 @@
#define GEN6_CONTEXT_ALIGN (6410)
#define GEN7_CONTEXT_ALIGN 4096
-static void do_ppgtt_cleanup(struct i915_hw_ppgtt *ppgtt)
-{
- struct drm_device *dev = ppgtt-base.dev;
- struct drm_i915_private *dev_priv = dev-dev_private;
- struct i915_address_space *vm = ppgtt-base;
-
- if (ppgtt == dev_priv-mm.aliasing_ppgtt ||
- (list_empty(vm-active_list)
list_empty(vm-inactive_list)))
{
- ppgtt-base.cleanup(ppgtt-base);
- return;
- }
-
- /* vmas should already be unbound */
- WARN_ON(!list_empty(vm-active_list));
-
- ppgtt-base.cleanup(ppgtt-base);
-}
-
-void ppgtt_release(struct kref *kref)
-{
- struct i915_hw_ppgtt *ppgtt =
- container_of(kref, struct i915_hw_ppgtt, ref);
-
- do_ppgtt_cleanup(ppgtt);
- kfree(ppgtt);
-}
-
static size_t get_context_alignment(struct drm_device *dev)
{
if (IS_GEN6(dev))
@@ -171,8 +144,7 @@ void i915_gem_context_free(struct kref *ctx_ref)
ppgtt = ctx_to_ppgtt(ctx);
}
- if (ppgtt)
- kref_put(ppgtt-ref, ppgtt_release);
+ i915_ppgtt_put(ppgtt);
if (ctx-legacy_hw_ctx.rcs_state)
drm_gem_object_unreference(ctx-
legacy_hw_ctx.rcs_state-base);
list_del(ctx-link);
@@ -219,7
Re: [Intel-gfx] [PATCH 05/15] drm/i915: Only refcount ppgtt if it actually is one
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
Of Daniel Vetter
Sent: Wednesday, August 06, 2014 2:05 PM
To: Intel Graphics Development
Cc: Daniel Vetter
Subject: [Intel-gfx] [PATCH 05/15] drm/i915: Only refcount ppgtt if it
actually
is one
This essentially unbreaks non-ppgtt operation where we'd scribble over
random memory.
While at it give the vm_to_ppgtt function a proper prefix and make it
a bit more paranoid.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_drv.h | 10 +-
drivers/gpu/drm/i915/i915_gem.c | 3 ++-
drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ++-
3 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index 194367f0ba1a..31422bc07445 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2475,6 +2475,15 @@ static inline bool i915_is_ggtt(struct
i915_address_space *vm)
return vm == ggtt;
}
+static inline struct i915_hw_ppgtt *
+i915_vm_to_ppgtt(struct i915_address_space *vm)
+{
+ WARN_ON(i915_is_ggtt(vm));
+
+ return container_of(vm, struct i915_hw_ppgtt, base);
+}
+
+
static inline bool i915_gem_obj_ggtt_bound(struct drm_i915_gem_object
*obj)
{
return i915_gem_obj_bound(obj, obj_to_ggtt(obj));
@@ -2510,7 +2519,6 @@ void i915_gem_object_ggtt_unpin(struct
drm_i915_gem_object *obj);
/* i915_gem_context.c */
#define ctx_to_ppgtt(ctx) container_of((ctx)-vm, struct i915_hw_ppgtt,
base)
-#define vm_to_ppgtt(vm) container_of(vm, struct i915_hw_ppgtt, base)
int __must_check i915_gem_context_init(struct drm_device *dev);
void i915_gem_context_fini(struct drm_device *dev);
void i915_gem_context_reset(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index b33a677b4b1e..c43ccfdf45a5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4508,7 +4508,8 @@ void i915_gem_vma_destroy(struct i915_vma
*vma)
vm = vma-vm;
- i915_ppgtt_put(vm_to_ppgtt(vm));
+ if (!i915_is_ggtt(vm))
+ i915_ppgtt_put(i915_vm_to_ppgtt(vm));
list_del(vma-vma_link);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 83ee41e5c1c7..3753bf184865 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2204,7 +2204,8 @@ i915_gem_obj_lookup_or_create_vma(struct
drm_i915_gem_object *obj,
if (!vma)
vma = __i915_gem_vma_create(obj, vm);
- i915_ppgtt_get(vm_to_ppgtt(vm));
+ if (!i915_is_ggtt(vm))
+ i915_ppgtt_get(i915_vm_to_ppgtt(vm));
return vma;
}
--
1.9.3
Reviewed-by: Michel Thierry michel.thie...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
smime.p7s
Description: S/MIME cryptographic signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Program PFI credits for VLV
On Thu, Aug 07, 2014 at 06:40:03PM +0530, Vandana Kannan wrote:
From: Vidya Srinivas vidya.srini...@intel.com
PFI credit programming is required when CD clock (related to data flow from
display pipeline to end display) is greater than CZ clock (related to data
flow from memory to display plane). This programming should be done when all
planes are OFF to avoid intermittent hangs while accessing memory even from
different Gfx units (not just display).
If cdclk/czclk =1, PFI credits could be set as any number. To get better
performance, larger PFI credit can be assigned to PND. Otherwise if
cdclk/czclk1, the default PFI credit of 8 should be set.
Do we have any docs for this? I see the register in the docs but nothing
about the correct programming. Some kind of refrence to the used
documentation would be nice.
v2:
- Change log to lower log level instead of DRM_ERROR
- Change function name to valleyview_program_pfi_credits
- Move program PFI credits to modeset_init instead of intel_set_mode
- Change magic numbers to logical constants
Signed-off-by: Vidya Srinivas vidya.srini...@intel.com
Signed-off-by: Gajanan Bhat gajanan.b...@intel.com
Signed-off-by: Vandana Kannan vandana.kan...@intel.com
---
drivers/gpu/drm/i915/i915_drv.c | 6 ++
drivers/gpu/drm/i915/i915_drv.h | 2 ++
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/intel_display.c | 4 +++-
drivers/gpu/drm/i915/intel_pm.c | 22 ++
5 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6c4b25c..00e0b4a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
console_unlock();
+ if (IS_VALLEYVIEW(dev))
+ valleyview_program_pfi_credits(dev_priv, false);
If we want to do this for system suspend I think we should stick it
into i915_drm_freeze() (just after turning off the pipes). Not sure if
we should also force cdclk to minimum there...
Hmm. Is the GCI_CONTROL register is part of the disp2d power well? If it
is we probably need to enable the power well around the register write.
+
dev_priv-suspend_count++;
intel_display_set_init_power(dev_priv, false);
@@ -693,6 +696,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
restore_gtt_mappings)
dev_priv-modeset_restore = MODESET_DONE;
mutex_unlock(dev_priv-modeset_restore_lock);
+ if (IS_VALLEYVIEW(dev))
+ valleyview_program_pfi_credits(dev_priv, true);
I think this thing can be dropped. We need to do the reprogramming as
part of the modeset global resources handling.
+
intel_opregion_notify_adapter(dev, PCI_D0);
return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 881e0a6..88fd4c7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2172,6 +2172,8 @@ extern struct i915_params i915 __read_mostly;
/* i915_dma.c */
void i915_update_dri1_breadcrumb(struct drm_device *dev);
+extern void valleyview_program_pfi_credits(struct drm_i915_private *dev_priv,
+bool flag);
extern void i915_kernel_lost_context(struct drm_device * dev);
extern int i915_driver_load(struct drm_device *, unsigned long flags);
extern int i915_driver_unload(struct drm_device *);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fb111cd..7f4c2f5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1937,6 +1937,11 @@ enum punit_power_well {
#define CZCLK_FREQ_MASK0xf
#define GMBUSFREQ_VLV(VLV_DISPLAY_BASE + 0x6510)
+#define GCI_CONTROL (VLV_DISPLAY_BASE + 0x650C)
+#define PFI_CREDIT (7 28)
Maybe something like this for a bit more self documenting code.
#define PFI_CREDIT(x) (((x)-8) 28) /* 8-15 */
+#define PFI_CREDIT_RESEND (1 27)
+#define VGA_FAST_MODE_DISABLE (1 14)
+
/*
* Palette regs
*/
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 2089319..2af2e60 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12691,8 +12691,10 @@ void intel_modeset_init_hw(struct drm_device *dev)
{
intel_prepare_ddi(dev);
- if (IS_VALLEYVIEW(dev))
+ if (IS_VALLEYVIEW(dev)) {
vlv_update_cdclk(dev);
+ valleyview_program_pfi_credits(dev-dev_private, true);
The planes may be be active here. So for the initial state we just need
to trust that the BIOS got it right.
For runtime cdclk changes this needs to be handled in
valleyview_modeset_global_resources().
Re: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for aliasing ppgtt
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
Of Daniel Vetter
Sent: Wednesday, August 06, 2014 2:05 PM
To: Intel Graphics Development
Cc: Daniel Vetter
Subject: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for aliasing
ppgtt
A subsequent patch will no longer initialize the aliasing ppgtt if we
have full ppgtt enabled, since we simply don't need that any more.
Unfortunately a few places check for the aliasing ppgtt instead of
checking for ppgtt in general. Fix them up.
One special case are the gtt offset and size macros, which have some
code to remap the aliasing ppgtt to the global gtt. The aliasing ppgtt
is _not_ a logical address space, so passing that in as the vm is
plain and simple a bug. So just WARN about it and carry on - we have a
gracefully fall-through anyway if we can't find the vma.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 4 +---
drivers/gpu/drm/i915/i915_dma.c | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 8 ++--
drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +---
4 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index dea99d92fb4a..c45856bcc8b9 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -842,8 +842,6 @@ finish:
*/
bool i915_needs_cmd_parser(struct intel_engine_cs *ring)
{
- struct drm_i915_private *dev_priv = ring-dev-dev_private;
-
if (!ring-needs_cmd_parser)
return false;
@@ -852,7 +850,7 @@ bool i915_needs_cmd_parser(struct intel_engine_cs
*ring)
* disabled. That will cause all of the parser's PPGTT checks to
* fail. For now, disable parsing when PPGTT is off.
*/
- if (!dev_priv-mm.aliasing_ppgtt)
+ if (USES_PPGTT(ring-dev))
return false;
return (i915.enable_cmd_parser == 1);
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index 2e7f03ad5ee2..e5ac1a6e9d26 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -999,7 +999,7 @@ static int i915_getparam(struct drm_device *dev, void
*data,
value = HAS_WT(dev);
break;
case I915_PARAM_HAS_ALIASING_PPGTT:
- value = dev_priv-mm.aliasing_ppgtt ||
USES_FULL_PPGTT(dev);
+ value = USES_PPGTT(dev);
break;
case I915_PARAM_HAS_WAIT_TIMEOUT:
value = 1;
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index d8399ee622b9..a79deb189146 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5114,9 +5114,7 @@ unsigned long i915_gem_obj_offset(struct
drm_i915_gem_object *o,
struct drm_i915_private *dev_priv = o-base.dev-dev_private;
struct i915_vma *vma;
- if (!dev_priv-mm.aliasing_ppgtt ||
- vm == dev_priv-mm.aliasing_ppgtt-base)
- vm = dev_priv-gtt.base;
+ WARN_ON(vm == dev_priv-mm.aliasing_ppgtt-base);
list_for_each_entry(vma, o-vma_list, vma_link) {
if (vma-vm == vm)
@@ -5157,9 +5155,7 @@ unsigned long i915_gem_obj_size(struct
drm_i915_gem_object *o,
struct drm_i915_private *dev_priv = o-base.dev-dev_private;
struct i915_vma *vma;
- if (!dev_priv-mm.aliasing_ppgtt ||
- vm == dev_priv-mm.aliasing_ppgtt-base)
- vm = dev_priv-gtt.base;
+ WARN_ON(vm == dev_priv-mm.aliasing_ppgtt-base);
BUG_ON(list_empty(o-vma_list));
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 05969f03c0c1..390e38325b37 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1977,9 +1977,7 @@ gen8_ring_dispatch_execbuffer(struct
intel_engine_cs *ring,
u64 offset, u32 len,
unsigned flags)
{
- struct drm_i915_private *dev_priv = ring-dev-dev_private;
- bool ppgtt = dev_priv-mm.aliasing_ppgtt != NULL
- !(flags I915_DISPATCH_SECURE);
+ bool ppgtt = USES_PPGTT(ring-dev) !(flags
I915_DISPATCH_SECURE);
int ret;
ret = intel_ring_begin(ring, 4);
--
1.9.3
Sorry, I spotted another instance in gen8_ppgtt_info (debugfs). It is more
gen8_ppgtt_info's fault, so I'm sending a follow up patch for it.
Unless you want to combine them...
Reviewed-by: Michel Thierry michel.thie...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
smime.p7s
Description: S/MIME cryptographic signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Initialize the aliasing ppgtt as part of global gtt
-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch]
Sent: Wednesday, August 06, 2014 7:20 PM
To: Intel Graphics Development
Cc: Daniel Vetter; Thierry, Michel; Ville Syrjälä
Subject: [PATCH 2/2] drm/i915: Initialize the aliasing ppgtt as part of
global gtt
Stuffing this into the context setup code doesn't make a lot of sense.
Also reusing the real ppgtt setup code makes even less sense since the
aliasing ppgtt isn't a real address space. Leaving all that stuff
unitialized will make sure that we catch any abusers promptly.
This is also a prep work to clean up the context-ppgtt link.
v2: Fix up the logic fail, I've fumbled it so badly to completely
disable ppgtt on gen6. Spotted by Ville and Michel. Also move around
the pde write into the gen6 init function, since otherwise it won't
work at all.
v3: Only initialize the aliasing ppgtt when we actually enable it.
Cc: Thierry, Michel michel.thie...@intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_context.c | 13 +-
drivers/gpu/drm/i915/i915_gem_gtt.c | 42 +++-
-
2 files changed, 31 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index 4af5f05b24e8..07f4d060575b 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -276,17 +276,6 @@ i915_gem_create_context(struct drm_device *dev,
goto err_unpin;
} else
ctx-vm = ppgtt-base;
-
- /* This case is reserved for the global default context and
- * should only happen once. */
- if (is_global_default_ctx) {
- if (WARN_ON(dev_priv-mm.aliasing_ppgtt)) {
- ret = -EEXIST;
- goto err_unpin;
- }
-
- dev_priv-mm.aliasing_ppgtt = ppgtt;
- }
} else if (USES_PPGTT(dev)) {
/* For platforms which only have aliasing PPGTT, we fake the
* address space and refcounting. */
@@ -361,7 +350,7 @@ int i915_gem_context_init(struct drm_device *dev)
}
}
- ctx = i915_gem_create_context(dev, NULL, USES_PPGTT(dev));
+ ctx = i915_gem_create_context(dev, NULL,
USES_FULL_PPGTT(dev));
if (IS_ERR(ctx)) {
DRM_ERROR(Failed to create default global context (error
%ld)\n,
PTR_ERR(ctx));
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index bf70ab44b968..653a1166a3fa 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1141,35 +1141,38 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt
*ppgtt)
ppgtt-node.size 20,
ppgtt-node.start / PAGE_SIZE);
+ gen6_write_pdes(ppgtt);
+ DRM_DEBUG(Adding PPGTT at offset %x\n,
+ ppgtt-pd_offset 10);
+
return 0;
}
-int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
+int __hw_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
{
struct drm_i915_private *dev_priv = dev-dev_private;
- int ret = 0;
ppgtt-base.dev = dev;
ppgtt-base.scratch = dev_priv-gtt.base.scratch;
if (INTEL_INFO(dev)-gen 8)
- ret = gen6_ppgtt_init(ppgtt);
+ return gen6_ppgtt_init(ppgtt);
else if (IS_GEN8(dev))
- ret = gen8_ppgtt_init(ppgtt, dev_priv-gtt.base.total);
+ return gen8_ppgtt_init(ppgtt, dev_priv-gtt.base.total);
else
BUG();
+}
+int i915_ppgtt_init(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ int ret = 0;
- if (!ret) {
- struct drm_i915_private *dev_priv = dev-dev_private;
+ ret = __hw_ppgtt_init(dev, ppgtt);
+ if (ret == 0) {
kref_init(ppgtt-ref);
drm_mm_init(ppgtt-base.mm, ppgtt-base.start,
ppgtt-base.total);
i915_init_vm(dev_priv, ppgtt-base);
- if (INTEL_INFO(dev)-gen 8) {
- gen6_write_pdes(ppgtt);
- DRM_DEBUG(Adding PPGTT at offset %x\n,
- ppgtt-pd_offset 10);
- }
}
return ret;
@@ -1710,6 +1713,7 @@ int i915_gem_setup_global_gtt(struct drm_device
*dev,
struct drm_mm_node *entry;
struct drm_i915_gem_object *obj;
unsigned long hole_start, hole_end;
+ int ret;
BUG_ON(mappable_end end);
@@ -1721,7 +1725,7 @@ int i915_gem_setup_global_gtt(struct drm_device
*dev,
/* Mark any preallocated objects as
Re: [Intel-gfx] [PATCH 11/15] drm/i915: Drop create_vm argument to i915_gem_create_context
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
Of Daniel Vetter
Sent: Wednesday, August 06, 2014 2:05 PM
To: Intel Graphics Development
Cc: Daniel Vetter
Subject: [Intel-gfx] [PATCH 11/15] drm/i915: Drop create_vm argument to
i915_gem_create_context
Now that all the flow is streamlined the rule is simple: We create
a new ppgtt for a new context when we have full ppgtt enabled.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_gem_context.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
b/drivers/gpu/drm/i915/i915_gem_context.c
index c00e5d027774..655ed6228aab 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -229,8 +229,7 @@ err_out:
*/
static struct intel_context *
i915_gem_create_context(struct drm_device *dev,
- struct drm_i915_file_private *file_priv,
- bool create_vm)
+ struct drm_i915_file_private *file_priv)
{
const bool is_global_default_ctx = file_priv == NULL;
struct intel_context *ctx;
@@ -258,7 +257,7 @@ i915_gem_create_context(struct drm_device *dev,
}
}
- if (create_vm) {
+ if (USES_FULL_PPGTT(dev)) {
struct i915_hw_ppgtt *ppgtt = i915_ppgtt_create(dev,
file_priv);
if (IS_ERR_OR_NULL(ppgtt)) {
@@ -337,7 +336,7 @@ int i915_gem_context_init(struct drm_device *dev)
}
}
- ctx = i915_gem_create_context(dev, NULL,
USES_FULL_PPGTT(dev));
+ ctx = i915_gem_create_context(dev, NULL);
if (IS_ERR(ctx)) {
DRM_ERROR(Failed to create default global context (error
%ld)\n,
PTR_ERR(ctx));
@@ -438,7 +437,7 @@ int i915_gem_context_open(struct drm_device *dev,
struct drm_file *file)
idr_init(file_priv-context_idr);
mutex_lock(dev-struct_mutex);
- ctx = i915_gem_create_context(dev, file_priv,
USES_FULL_PPGTT(dev));
+ ctx = i915_gem_create_context(dev, file_priv);
mutex_unlock(dev-struct_mutex);
if (IS_ERR(ctx)) {
@@ -696,7 +695,7 @@ int i915_gem_context_create_ioctl(struct drm_device
*dev, void *data,
if (ret)
return ret;
- ctx = i915_gem_create_context(dev, file_priv,
USES_FULL_PPGTT(dev));
+ ctx = i915_gem_create_context(dev, file_priv);
mutex_unlock(dev-struct_mutex);
if (IS_ERR(ctx))
return PTR_ERR(ctx);
--
1.9.3
Reviewed-by: Michel Thierry michel.thie...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
smime.p7s
Description: S/MIME cryptographic signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 12/15] drm/i915: Extract common cleanup into i915_ppgtt_release
-Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Daniel Vetter Sent: Wednesday, August 06, 2014 2:05 PM To: Intel Graphics Development Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH 12/15] drm/i915: Extract common cleanup into i915_ppgtt_release Address space cleanup isn't really a job for the low-level cleanup callbacks. Without this change we can't reuse the low-level cleanup callback for the aliasing ppgtt cleanup. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_gem_gtt.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index a4c1740d79be..bbf113ed7339 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -403,9 +403,6 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm) struct i915_hw_ppgtt *ppgtt = container_of(vm, struct i915_hw_ppgtt, base); - list_del(vm-global_link); - drm_mm_takedown(vm-mm); - gen8_ppgtt_unmap_pages(ppgtt); gen8_ppgtt_free(ppgtt); } @@ -1029,8 +1026,6 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm) struct i915_hw_ppgtt *ppgtt = container_of(vm, struct i915_hw_ppgtt, base); - list_del(vm-global_link); - drm_mm_takedown(ppgtt-base.mm); drm_mm_remove_node(ppgtt-node); gen6_ppgtt_unmap_pages(ppgtt); @@ -1255,6 +1250,9 @@ void i915_ppgtt_release(struct kref *kref) WARN_ON(!list_empty(ppgtt-base.active_list)); WARN_ON(!list_empty(ppgtt-base.inactive_list)); + list_del(ppgtt-base.global_link); + drm_mm_takedown(ppgtt-base.mm); + ppgtt-base.cleanup(ppgtt-base); kfree(ppgtt); } -- 1.9.3 Reviewed-by: Michel Thierry michel.thie...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 14/15] drm/i915: Cleanup aliasging ppgtt alongside the global gtt
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
Of Daniel Vetter
Sent: Wednesday, August 06, 2014 2:05 PM
To: Intel Graphics Development
Cc: Daniel Vetter
Subject: [Intel-gfx] [PATCH 14/15] drm/i915: Cleanup aliasging ppgtt
alongside the global gtt
Also remove related WARN_ONs which seem to have been hit since a rather
long time. But apperently no one noticed since our module reload is
already WARNING-infested :(
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_dma.c | 4
drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index c176a6c97c80..94afe7c4458b 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1388,7 +1388,6 @@ cleanup_gem:
i915_gem_cleanup_ringbuffer(dev);
i915_gem_context_fini(dev);
mutex_unlock(dev-struct_mutex);
- WARN_ON(dev_priv-mm.aliasing_ppgtt);
cleanup_irq:
drm_irq_uninstall(dev);
cleanup_gem_stolen:
@@ -1897,7 +1896,6 @@ int i915_driver_unload(struct drm_device *dev)
mutex_lock(dev-struct_mutex);
i915_gem_cleanup_ringbuffer(dev);
i915_gem_context_fini(dev);
- WARN_ON(dev_priv-mm.aliasing_ppgtt);
mutex_unlock(dev-struct_mutex);
i915_gem_cleanup_stolen(dev);
@@ -1905,8 +1903,6 @@ int i915_driver_unload(struct drm_device *dev)
i915_free_hws(dev);
}
- WARN_ON(!list_empty(dev_priv-vm_list));
-
drm_vblank_cleanup(dev);
intel_teardown_gmbus(dev);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 2eab0b6a32e8..ff031bb1f296 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1801,6 +1801,12 @@ void i915_global_gtt_cleanup(struct drm_device
*dev)
struct drm_i915_private *dev_priv = dev-dev_private;
struct i915_address_space *vm = dev_priv-gtt.base;
+ if (dev_priv-mm.aliasing_ppgtt) {
+ struct i915_hw_ppgtt *ppgtt = dev_priv-mm.aliasing_ppgtt;
+
+ ppgtt-base.cleanup(ppgtt-base);
+ }
+
if (drm_mm_initialized(vm-mm)) {
drm_mm_takedown(vm-mm);
list_del(vm-global_link);
@@ -1808,6 +1814,7 @@ void i915_global_gtt_cleanup(struct drm_device
*dev)
vm-cleanup(vm);
}
+
static int setup_scratch_page(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
--
1.9.3
Reviewed-by: Michel Thierry michel.thie...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
smime.p7s
Description: S/MIME cryptographic signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Update gen8_ppgtt_info to keep working in full ppgtt
The driver will no longer initialize the aliasing ppgtt if we have full ppgtt enabled. gen8_ppgtt_info uses the aliasing ppgtt or the ppgtt from the default context. This patch makes it clear. Cc: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Michel Thierry michel.thie...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 8dc82c3..a62ac66 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1995,8 +1995,14 @@ static void gen8_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 i915_hw_ppgtt *ppgtt = dev_priv-mm.aliasing_ppgtt; + struct intel_context *ctx = dev_priv-ring[RCS].default_context; int unused, i; + if (USES_FULL_PPGTT(dev)) + ppgtt = ctx-ppgtt; + else + ppgtt = dev_priv-mm.aliasing_ppgtt; + if (!ppgtt) return; -- 2.0.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/15] drm/i915: Extract commmon global gtt cleanup code
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
Of Daniel Vetter
Sent: Wednesday, August 06, 2014 2:05 PM
To: Intel Graphics Development
Cc: Daniel Vetter
Subject: [Intel-gfx] [PATCH 13/15] drm/i915: Extract commmon global gtt
cleanup code
We want to move the aliasing ppgtt cleanup back into the global
gtt cleanup code for symmetric, but first we need to create such
a place.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_dma.c | 4 ++--
drivers/gpu/drm/i915/i915_gem_gtt.c | 20
drivers/gpu/drm/i915/i915_gem_gtt.h | 1 +
3 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index e5ac1a6e9d26..c176a6c97c80 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1817,7 +1817,7 @@ out_mtrrfree:
arch_phys_wc_del(dev_priv-gtt.mtrr);
io_mapping_free(dev_priv-gtt.mappable);
out_gtt:
- dev_priv-gtt.base.cleanup(dev_priv-gtt.base);
+ i915_global_gtt_cleanup(dev);
out_regs:
intel_uncore_fini(dev);
pci_iounmap(dev-pdev, dev_priv-regs);
@@ -1916,7 +1916,7 @@ int i915_driver_unload(struct drm_device *dev)
destroy_workqueue(dev_priv-wq);
pm_qos_remove_request(dev_priv-pm_qos);
- dev_priv-gtt.base.cleanup(dev_priv-gtt.base);
+ i915_global_gtt_cleanup(dev);
intel_uncore_fini(dev);
if (dev_priv-regs != NULL)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index bbf113ed7339..2eab0b6a32e8 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1796,6 +1796,18 @@ void i915_gem_init_global_gtt(struct drm_device
*dev)
i915_gem_setup_global_gtt(dev, 0, mappable_size, gtt_size);
}
+void i915_global_gtt_cleanup(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct i915_address_space *vm = dev_priv-gtt.base;
+
+ if (drm_mm_initialized(vm-mm)) {
+ drm_mm_takedown(vm-mm);
+ list_del(vm-global_link);
+ }
+
+ vm-cleanup(vm);
+}
static int setup_scratch_page(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -2064,10 +2076,6 @@ static void gen6_gmch_remove(struct
i915_address_space *vm)
struct i915_gtt *gtt = container_of(vm, struct i915_gtt, base);
- if (drm_mm_initialized(vm-mm)) {
- drm_mm_takedown(vm-mm);
- list_del(vm-global_link);
- }
iounmap(gtt-gsm);
teardown_scratch_page(vm-dev);
}
@@ -2106,10 +2114,6 @@ static int i915_gmch_probe(struct drm_device
*dev,
static void i915_gmch_remove(struct i915_address_space *vm)
{
- if (drm_mm_initialized(vm-mm)) {
- drm_mm_takedown(vm-mm);
- list_del(vm-global_link);
- }
intel_gmch_remove();
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h
b/drivers/gpu/drm/i915/i915_gem_gtt.h
index bea3541d5525..6b30ebad0a0a 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -273,6 +273,7 @@ int i915_gem_gtt_init(struct drm_device *dev);
void i915_gem_init_global_gtt(struct drm_device *dev);
int i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long
start,
unsigned long mappable_end, unsigned long
end);
+void i915_global_gtt_cleanup(struct drm_device *dev);
bool intel_enable_ppgtt(struct drm_device *dev, bool full);
--
1.9.3
Reviewed-by: Michel Thierry michel.thie...@intel.com
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
smime.p7s
Description: S/MIME cryptographic signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC] drm/i915/bdw: Apply workarounds to the golden render state
On 08/08/2014 13:20, Ville Syrjälä wrote: On Fri, Aug 08, 2014 at 10:52:57AM +0100, arun.siluv...@linux.intel.com wrote: From: Arun Siluvery arun.siluv...@linux.intel.com Workarounds for bdw are currently applied in init_clock_gating() but they are lost following a gpu reset. Some of the registers are part of register state context and they are restored with every context switch so initializing WAs in golden render state ensures that they are applied even when we start with an uninitialized context or during hw initialization followed by a reset. This approach might require separate null states for BDW vs. CHV and IVB vs. HSW vs. VLV, which seems a bit unfortunate. Might be better to just issue the w/a register writes via LRIs from the code as part of the null state load. Yes this is a better approach, I am currently changing the code to achieve this, not sure how easy it would be. Although I don't actually undertand how this improves things as opposed to just appllying the w/as via mmio writes. Does it? I observed random behaviour CACHE_MODE_1 which simply used to lose the applied workaround on first context switch even though it is loaded with inhibit==1; register values are not supposed to change but it was changing. I think it is better to add them in null batch to ensure hardware starts with WAs applied. regards Arun Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/intel_pm.c | 50 - drivers/gpu/drm/i915/intel_renderstate_gen8.c | 62 +-- 2 files changed, 39 insertions(+), 73 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 1ddd4df..ab64b64 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5402,38 +5402,11 @@ static void gen8_init_clock_gating(struct drm_device *dev) /* FIXME(BDW): Check all the w/a, some might only apply to * pre-production hw. */ - /* WaDisablePartialInstShootdown:bdw */ - I915_WRITE(GEN8_ROW_CHICKEN, - _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE)); - - /* WaDisableThreadStallDopClockGating:bdw */ - /* FIXME: Unclear whether we really need this on production bdw. */ - I915_WRITE(GEN8_ROW_CHICKEN, - _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE)); - - /* -* This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for -* pre-production hardware -*/ - I915_WRITE(HALF_SLICE_CHICKEN3, - _MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS)); - I915_WRITE(HALF_SLICE_CHICKEN3, - _MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS)); I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_BWGTLB_DISABLE)); I915_WRITE(_3D_CHICKEN3, _MASKED_BIT_ENABLE(_3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(2))); - I915_WRITE(COMMON_SLICE_CHICKEN2, - _MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE)); - - I915_WRITE(GEN7_HALF_SLICE_CHICKEN1, - _MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE)); - - /* WaDisableDopClockGating:bdw May not be needed for production */ - I915_WRITE(GEN7_ROW_CHICKEN2, - _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); - /* WaSwitchSolVfFArbitrationPriority:bdw */ I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL); @@ -5448,41 +5421,18 @@ static void gen8_init_clock_gating(struct drm_device *dev) BDW_DPRS_MASK_VBLANK_SRD); } - /* Use Force Non-Coherent whenever executing a 3D context. This is a -* workaround for for a possible hang in the unlikely event a TLB -* invalidation occurs during a PSD flush. -*/ - I915_WRITE(HDC_CHICKEN0, - I915_READ(HDC_CHICKEN0) | - _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT)); - /* WaVSRefCountFullforceMissDisable:bdw */ /* WaDSRefCountFullforceMissDisable:bdw */ I915_WRITE(GEN7_FF_THREAD_MODE, I915_READ(GEN7_FF_THREAD_MODE) ~(GEN8_FF_DS_REF_CNT_FFME | GEN7_FF_VS_REF_CNT_FFME)); - /* -* BSpec recommends 8x4 when MSAA is used, -* however in practice 16x4 seems fastest. -* -* Note that PS/WM thread counts depend on the WIZ hashing -* disable bit, which we don't touch here, but it's good -* to keep in mind (see 3DSTATE_PS and 3DSTATE_WM). -*/ - I915_WRITE(GEN7_GT_MODE, - GEN6_WIZ_HASHING_MASK | GEN6_WIZ_HASHING_16x4); - I915_WRITE(GEN6_RC_SLEEP_PSMI_CONTROL, _MASKED_BIT_ENABLE(GEN8_RC_SEMA_IDLE_MSG_DISABLE)); /* WaDisableSDEUnitClockGating:bdw */ I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) | GEN8_SDEUNIT_CLOCK_GATE_DISABLE); - -
Re: [Intel-gfx] Usage of _PAGE_PCD et al in i915 driver
Adding relevant mailing lists. On Fri, Aug 8, 2014 at 1:23 PM, Juergen Gross jgr...@suse.com wrote: I'm just about to create a patch for full PAT support in the Linux kernel, including Xen. For this purpose I introduce a translation between cache modes and pte bits. Scanning the kernel sources for usage of the cache mode bits in the pte I discovered drivers/gpu/drm/i915/i915_gem_gtt.h is using _PAGE_PCD, _PAGE_PWT and _PAGE_PAT. I think those defines are used to create ptes not for usage by the main processor, but for the graphics processor. Is this true? In this case I'd suggest to define i915-specific macros instead of using the x86 ones. Yeah, those are gpu specific PAT tables, but the hw engineers specifically designed this to match, and we've tried to follow the cpu side to match it. Especially in the future that will be somewhat important, since we want to fully share the entire address space between cpu and gpu on the next platform. Jesse is working on that. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm: Don't grab an fb reference for the idr
On Fri, Aug 8, 2014 at 12:35 PM, David Herrmann dh.herrm...@gmail.com wrote: Ewww, this is ugly. You now make the unregistration dynamic and it's no longer under driver control. The typical device-control flow assumes there's an authority that controls at which point objects are registered and unregistered. You now bind it to ref-counts. To be fair, I think lastclose is equally hackish (and doesn't really work like you argued already). Nope, it don't bind the unregister to the refcount, I only make the registerstration weak so that you can do such games. For all the normal framebuffers and anything controlled by drivers we can still unregister at any point before the final unref. I think the real problem is that you want two ref-counts: One ref-count controls the object availability, the other ref-count controls the visibility to user-space. This is also what gem does: you have a kernel-internal ref-count for each gem-object, but you also have user-space handles. A handle implies a kernel-internal ref-count but not vice versa. And the object is only accessible from user-space, as long as you own a handle. I think we want something similar for FBs. This way you could unregister the bios-FB once no handle exists (assuming a CRTC owns a handle as long as the FB is used as scan-out). But I guess no-one wants to implement this, so I still think this patch is the nicest way to make it work. So I'm fine with it. This doesn't actually solve anything here - the problem is exactly that this fb just exists, with no one having an access handle (beyond the internal usage refcount) on it. Not the fbdev emulation, not anything from userspace. But we want userspace to get at it with a gettfb call for smooth transitions, so simply not registering it also won't work. The thing just hangs of a few crtcs and should survive as long as that's the case, but as soon as it's no longer used it should get reaped. So the right place to hook the unregistration into is really when the last refcount goes away. -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] Odd behavior change in HD 4600/i915 graphics with Fedora 20
On Mon, 07 Jul 2014, Paul Livingston p...@accokeekwoods.com wrote: I recently built a new desktop machine using a Shuttle SZ87R6, an I7-4770S and a Dell U2713HM monitor. Did my initial HW burn-in and testing using a Fedora 20 KDE Spin Live CD (Kernel 3.11.10-301.fc20.x86_64 KDE Systems settings v4.11.3) using the DVI port on the computer and display and a dual link DVD-D cable, everything worked fine and the display resolution was 2560x1440 (native for the display). Both the monitor OSD and the KDE Systems Setting applet correctly reported both the resolution and the port DVI. The last of the parts came in (HD for secondary storage) and I finished the build and did an ion disk nstall and a full system update (Kernel 3.14.8-200.fc20.x86_64 and KDE System Settings v4.11.10, along with ~=750MB of other updates) The display looked odd after rebooting and when I checked it was set to 1600x1200 (not only wrong resolution but wrong aspect ratio) and the KDE applet now reported that the interface was HDMI vice the actual DVI. Checked the U2713HM OSD and found that while the max resolution was 2560x1440, current resolution was 1600x1200 and the interface was correctly reported (DVI-D). Verified the interconnection was DVI-D to DVI-D via a dual link DVI cable. Went back inside the OS and rechecked the Display and Monitor settings in Systems Settings and maximum available resolution was 1600x1200 (which is what the display was set to). also noted that the Display and Monitor applet was reporting an HDMI interconnect vice the actual DVI-D. Ran xrandr from the command line and found that while 1920x1200 was shown as an available resolution (at least the correct aspect ratio), I could not set it to that using the command line. Scratched my head and spent several hours Googling to see if anyone else had run into this problem - no luck, although there were some postings from roughly a year ago about people in the Win8 world having video problems with the HD 4600 IGA. I then spent some time on the Intel si te , found several conflicting threads on HD 4600 capability and did an explicit update to the latest version of the Intel video stack for Linux. No change and still unable to reset video resolution or aspect ratio from within F20. BTW, going back to the earlier kernel is not really a viable option as I wanted the privilege escalation bug fix that came out in 3.14.6, however with the earlier kernel both the resolution and aspect ratio is correct. At this stage, I have pretty well established (at least in my own mind) that it is a SW vice a HW problem and it is entirely reproducible as when I reboot with the Live CD the 2560x1440 resolution comes up by default. While at this point I rather strongly suspect the Intel i915 driver and video stack, it could also be either a KDE or an xorg or a kernel issue and I really don' know where to file a bug report since I can't nail down which SW is screwing up or what to try next. Seems odd that Intel would disable a HW capability that was working just fine as part of a driver update, not sure what the rationale for that would be Any ideas, insight or suggestions would be most welcome. https://bugzilla.kernel.org/show_bug.cgi?id=72961 BR, Jani. Thanks Paul Livingston p...@accokeekwoods.com ___ 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 45/53] drm/i915/bdw: Do not call intel_runtime_pm_get() in an interrupt
On Fri, Aug 08, 2014 at 11:37:01AM +0200, Daniel Vetter wrote:
On Fri, Aug 08, 2014 at 10:20:40AM +0100, Chris Wilson wrote:
On Tue, Jul 29, 2014 at 12:26:36PM +0200, Daniel Vetter wrote:
On Tue, Jul 29, 2014 at 08:37:48AM +0100, Chris Wilson wrote:
On Mon, Jul 28, 2014 at 10:54:06AM +0200, Daniel Vetter wrote:
On Sat, Jul 26, 2014 at 11:27:38AM +0100, Chris Wilson wrote:
On Wed, Jun 18, 2014 at 10:54:13PM +0200, Daniel Vetter wrote:
On Fri, Jun 13, 2014 at 04:38:03PM +0100, oscar.ma...@intel.com
wrote:
From: Oscar Mateo oscar.ma...@intel.com
Or with a spinlock grabbed, because it might sleep, which is not
a nice thing to do. Instead, do the runtime_pm get/put together
with the create/destroy request, and handle the forcewake
get/put
directly.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
Looks like a fixup that should be squashed into relevant earlier
patches.
The whole gen6_gt_force_wake_get() calling intel_runtime_pm_get() is
broken due to this - we must be able to read registers in atomic
context!
Please revert c8c8fb33b37766acf6474784b0d5245dab9a1690
force_wake_get can't call runtime_pm_get becuase pm_get can sleep. So
if
you want to read registers from atomic context you have to have a
runtime
pm reference from someone else.
Nope. That cannot work.
Well it works currently. So where do you see the problem?
Sampling registers from an timer - in particular, we really do not want
to disable runtime pm whilst trying to monitor the impact of runtime pm.
In that case you can grab a runtime pm reference iff the device is powered
on already. Which won't call anything scary, just amounts to an
atomic_add_unless or so, and then drop it again.
Unfortunately there doesn't seem to be such a thing around already, so
need to add it first. Greg, how much would you freak out if we add
something like
/**
* pm_runtime_get_unless_suspended - grab a rpm ref if the device is on
*
* Returns true if an rpm ref has been acquire, false otherwise. Can be
* called from atomic context to e.g. sample perfomance counters (where we
* obviously don't want to disturb system state if everything is off atm).
*/
static inline bool pm_runtime_get_unless_suspended(struct device *dev)
{
return atomic_add_unless(dev-power.usage_count, 1, 0);
}
I'd freak out a lot :)
Rafael, isn't there some other better way to resolve this issue without
resorting to something as horrid as the above?
thanks,
greg k-h
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.
On Fri, Aug 08, 2014 at 02:34:37PM +0300, Imre Deak wrote: On Fri, 2014-08-08 at 15:54 +0530, Sagar Arun Kamble wrote: On Fri, 2014-08-08 at 11:15 +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 02:29:38PM +0530, Sagar Arun Kamble wrote: On Fri, 2014-08-08 at 09:42 +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 12:22:44PM +0530, Sagar Arun Kamble wrote: Hi Daniel, On Mon, 2014-08-04 at 10:07 +0200, Daniel Vetter wrote: On Fri, Aug 01, 2014 at 12:34:56PM +0530, sagar.a.kam...@intel.com wrote: From: Sagar Kamble sagar.a.kam...@intel.com @@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev) intel_display_set_init_power(dev_priv, false); - return 0; + /* Save Gunit State and clear wake - Need to make sure +* changes in vlv_runtime_suspend path don't impact this path */ + if (IS_VALLEYVIEW(dev)) + ret = vlv_runtime_suspend(dev_priv); Maybe I wasn't clear, but I absolutely don't want any IS_VLV additions to core resume/thaw code. This should be shovelled into the runtime pm handling code, which should be reused in the suspend/resume code. This piece of code does not fit into any of the power well get/put path. Its specific sequence that need to be followed in VLV when Gunit gets power gated. So we have to keep this IS_VLV related functionality in both runtime and pm suspend/resume. Well we support S0ix now. Which means our system suspend/resume code actually calls into the runtime pm code. So either that design is broken (and we need to fix this) or something else is amiss. Or we don't need this code any more. But duplicating it is not the right approach. And yeah the power domain stuff might not be the right place, I've just used that as a place-holder for all the runtime pm code we have. -Daniel While entering S0iX we are getting following call flow: intel_runtime_resume followed by i915_drm_freeze and While resuming back from S0iX: i915_drm_thaw How can we share that wake control, gunit save/restore functionality? I am observing issue given below: Here is how I am achieving S0ix: 1. echo mem /sys/power/state 2. (Assuming we have removed all wake_locks in /sys/powe/wake_lock) If device is runtime suspend it is getting resumed with runtime_resume 3. PM core triggers this sequence for each device (prepare, suspend, suspend_late, suspend_noirq) 4. Then pm_suspend happens for Gfx. After all devices are suspended S0i0-S0iR-S0i1-S0i2-S0i3 happens. Gunit gets power gated. While resuming back (with user action/power button press) 1. PM core triggers following sequenc for each device: (resume_noirq, resume_early, resume, complete) After call to i915_drm_thaw_early(called through resume_early), during i915_drm_thaw(called through resume) we were seeing following issue during ring initialization. This is happening since wake is not enabled and Gunit registers are not restored(which is done in runtime_resume but that path is not taken here since this is resume from pm_suspend) We have intel_runtime_pm_get/put calls in the suspend/resume paths which should achieve this. Maybe they're not at the right place or not in the right ordering, but they're there. So on platforms with runtime pm support we _do_ call all the relevant runtime pm callbacks from system suspend/resume. -Daniel I am seeing following commit removed get/put calls from suspend resume paths: commit 395a5abbd97d7d76a7a26da52f33daebe279b3b3 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Wed Jun 18 09:52:56 2014 -0700 drm/i915: don't take runtime PM reference around freeze/thaw That ordering of get/put was also incorrect probably. Even though we do rpm_get before freeze after S0i3 we need to explicitely do wake contro/gunit save restore. In the current patch, the rpm handlers are called directly instead of (intel_runtime_pm_get/put). valleyview_runtime_suspend at the end of i915_drm_freeze and valleyview_runtime_resume at the beginning of i915_drm_thaw_early. Do I need to replace them with intel_display_power_get/put? Will DPM core allow rpm calls when system pm suspend/resume is in progress? No, you can't make the device runtime suspend while the system suspend handler runs since DPM takes an RPM reference before calling this handler. The reference will be dropped only after returning from the corresponding system resume handler. Blergh, I always forget that little piece of nonsense. Then we just need to call the relevant stuff directly, but not the
Re: [Intel-gfx] [PATCH] drm/i915: Update gen8_ppgtt_info to keep working in full ppgtt
On Fri, Aug 08, 2014 at 02:10:32PM +0100, Michel Thierry wrote: The driver will no longer initialize the aliasing ppgtt if we have full ppgtt enabled. gen8_ppgtt_info uses the aliasing ppgtt or the ppgtt from the default context. This patch makes it clear. Cc: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Michel Thierry michel.thie...@intel.com I actually looked at this one here and decided that ppgtt_info for gen8 is sufficiently broken so that we don't care. Imo we should refactor it a bit so that it does the same as the gen6 version, i.e. dump the aliasing ppgtt or the contexts. Trying to dump the default ctx ppgtt when full ppgtt is enabled is fairly usesless since nothing at all will ever use that one. Best approach is probably to remove the gen8 version and push the GenX checks down into the actual ppgtt dump functions or something like that. -Daniel --- drivers/gpu/drm/i915/i915_debugfs.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 8dc82c3..a62ac66 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1995,8 +1995,14 @@ static void gen8_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 i915_hw_ppgtt *ppgtt = dev_priv-mm.aliasing_ppgtt; + struct intel_context *ctx = dev_priv-ring[RCS].default_context; int unused, i; + if (USES_FULL_PPGTT(dev)) + ppgtt = ctx-ppgtt; + else + ppgtt = dev_priv-mm.aliasing_ppgtt; + if (!ppgtt) return; -- 2.0.3 -- 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 08/15] drm/i915: Fix up checks for aliasing ppgtt
On Fri, Aug 08, 2014 at 01:03:53PM +, Thierry, Michel wrote:
-Original Message-
From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
Of Daniel Vetter
Sent: Wednesday, August 06, 2014 2:05 PM
To: Intel Graphics Development
Cc: Daniel Vetter
Subject: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for aliasing
ppgtt
A subsequent patch will no longer initialize the aliasing ppgtt if we
have full ppgtt enabled, since we simply don't need that any more.
Unfortunately a few places check for the aliasing ppgtt instead of
checking for ppgtt in general. Fix them up.
One special case are the gtt offset and size macros, which have some
code to remap the aliasing ppgtt to the global gtt. The aliasing ppgtt
is _not_ a logical address space, so passing that in as the vm is
plain and simple a bug. So just WARN about it and carry on - we have a
gracefully fall-through anyway if we can't find the vma.
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/i915_cmd_parser.c | 4 +---
drivers/gpu/drm/i915/i915_dma.c | 2 +-
drivers/gpu/drm/i915/i915_gem.c | 8 ++--
drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +---
4 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c
b/drivers/gpu/drm/i915/i915_cmd_parser.c
index dea99d92fb4a..c45856bcc8b9 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -842,8 +842,6 @@ finish:
*/
bool i915_needs_cmd_parser(struct intel_engine_cs *ring)
{
- struct drm_i915_private *dev_priv = ring-dev-dev_private;
-
if (!ring-needs_cmd_parser)
return false;
@@ -852,7 +850,7 @@ bool i915_needs_cmd_parser(struct intel_engine_cs
*ring)
* disabled. That will cause all of the parser's PPGTT checks to
* fail. For now, disable parsing when PPGTT is off.
*/
- if (!dev_priv-mm.aliasing_ppgtt)
+ if (USES_PPGTT(ring-dev))
return false;
return (i915.enable_cmd_parser == 1);
diff --git a/drivers/gpu/drm/i915/i915_dma.c
b/drivers/gpu/drm/i915/i915_dma.c
index 2e7f03ad5ee2..e5ac1a6e9d26 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -999,7 +999,7 @@ static int i915_getparam(struct drm_device *dev, void
*data,
value = HAS_WT(dev);
break;
case I915_PARAM_HAS_ALIASING_PPGTT:
- value = dev_priv-mm.aliasing_ppgtt ||
USES_FULL_PPGTT(dev);
+ value = USES_PPGTT(dev);
break;
case I915_PARAM_HAS_WAIT_TIMEOUT:
value = 1;
diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index d8399ee622b9..a79deb189146 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5114,9 +5114,7 @@ unsigned long i915_gem_obj_offset(struct
drm_i915_gem_object *o,
struct drm_i915_private *dev_priv = o-base.dev-dev_private;
struct i915_vma *vma;
- if (!dev_priv-mm.aliasing_ppgtt ||
- vm == dev_priv-mm.aliasing_ppgtt-base)
- vm = dev_priv-gtt.base;
+ WARN_ON(vm == dev_priv-mm.aliasing_ppgtt-base);
list_for_each_entry(vma, o-vma_list, vma_link) {
if (vma-vm == vm)
@@ -5157,9 +5155,7 @@ unsigned long i915_gem_obj_size(struct
drm_i915_gem_object *o,
struct drm_i915_private *dev_priv = o-base.dev-dev_private;
struct i915_vma *vma;
- if (!dev_priv-mm.aliasing_ppgtt ||
- vm == dev_priv-mm.aliasing_ppgtt-base)
- vm = dev_priv-gtt.base;
+ WARN_ON(vm == dev_priv-mm.aliasing_ppgtt-base);
BUG_ON(list_empty(o-vma_list));
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 05969f03c0c1..390e38325b37 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1977,9 +1977,7 @@ gen8_ring_dispatch_execbuffer(struct
intel_engine_cs *ring,
u64 offset, u32 len,
unsigned flags)
{
- struct drm_i915_private *dev_priv = ring-dev-dev_private;
- bool ppgtt = dev_priv-mm.aliasing_ppgtt != NULL
- !(flags I915_DISPATCH_SECURE);
+ bool ppgtt = USES_PPGTT(ring-dev) !(flags
I915_DISPATCH_SECURE);
int ret;
ret = intel_ring_begin(ring, 4);
--
1.9.3
Sorry, I spotted another instance in gen8_ppgtt_info (debugfs). It is more
gen8_ppgtt_info's fault, so I'm sending a follow up patch for it.
Unless you want to combine them...
Reviewed-by: Michel Thierry michel.thie...@intel.com
Actually I spotted that one and decided that I can't break things worse
than it is - the aliasing ppgtt for full ppgtt is completely unused
(except for these checks), so dumping it doesn't add value.
To check: Does your r-b apply here
Re: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for aliasing ppgtt
-Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Friday, August 08, 2014 2:49 PM To: Thierry, Michel Cc: Daniel Vetter; Intel Graphics Development Subject: Re: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for aliasing ppgtt On Fri, Aug 08, 2014 at 01:03:53PM +, Thierry, Michel wrote: -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Daniel Vetter Sent: Wednesday, August 06, 2014 2:05 PM To: Intel Graphics Development Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH 08/15] drm/i915: Fix up checks for aliasing ppgtt A subsequent patch will no longer initialize the aliasing ppgtt if we have full ppgtt enabled, since we simply don't need that any more. Unfortunately a few places check for the aliasing ppgtt instead of checking for ppgtt in general. Fix them up. One special case are the gtt offset and size macros, which have some code to remap the aliasing ppgtt to the global gtt. The aliasing ppgtt is _not_ a logical address space, so passing that in as the vm is plain and simple a bug. So just WARN about it and carry on - we have a gracefully fall-through anyway if we can't find the vma. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/i915_cmd_parser.c | 4 +--- drivers/gpu/drm/i915/i915_dma.c | 2 +- drivers/gpu/drm/i915/i915_gem.c | 8 ++-- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 +--- 4 files changed, 5 insertions(+), 13 deletions(-) -- 1.9.3 Sorry, I spotted another instance in gen8_ppgtt_info (debugfs). It is more gen8_ppgtt_info's fault, so I'm sending a follow up patch for it. Unless you want to combine them... Reviewed-by: Michel Thierry michel.thie...@intel.com Actually I spotted that one and decided that I can't break things worse than it is - the aliasing ppgtt for full ppgtt is completely unused (except for these checks), so dumping it doesn't add value. To check: Does your r-b apply here still even without any fixup for gen8 ppgtt_info? -Daniel Yes, the r-b still applies. And you're also right about gen8_ppgtt_info, it wouldn't print anything useful in its current state. -Michel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.
Gn Fri, Aug 08, 2014 at 03:43:49PM +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 02:34:37PM +0300, Imre Deak wrote: On Fri, 2014-08-08 at 15:54 +0530, Sagar Arun Kamble wrote: On Fri, 2014-08-08 at 11:15 +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 02:29:38PM +0530, Sagar Arun Kamble wrote: On Fri, 2014-08-08 at 09:42 +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 12:22:44PM +0530, Sagar Arun Kamble wrote: Hi Daniel, On Mon, 2014-08-04 at 10:07 +0200, Daniel Vetter wrote: On Fri, Aug 01, 2014 at 12:34:56PM +0530, sagar.a.kam...@intel.com wrote: From: Sagar Kamble sagar.a.kam...@intel.com @@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev) intel_display_set_init_power(dev_priv, false); - return 0; + /* Save Gunit State and clear wake - Need to make sure + * changes in vlv_runtime_suspend path don't impact this path */ + if (IS_VALLEYVIEW(dev)) + ret = vlv_runtime_suspend(dev_priv); Maybe I wasn't clear, but I absolutely don't want any IS_VLV additions to core resume/thaw code. This should be shovelled into the runtime pm handling code, which should be reused in the suspend/resume code. This piece of code does not fit into any of the power well get/put path. Its specific sequence that need to be followed in VLV when Gunit gets power gated. So we have to keep this IS_VLV related functionality in both runtime and pm suspend/resume. Well we support S0ix now. Which means our system suspend/resume code actually calls into the runtime pm code. So either that design is broken (and we need to fix this) or something else is amiss. Or we don't need this code any more. But duplicating it is not the right approach. And yeah the power domain stuff might not be the right place, I've just used that as a place-holder for all the runtime pm code we have. -Daniel While entering S0iX we are getting following call flow: intel_runtime_resume followed by i915_drm_freeze and While resuming back from S0iX: i915_drm_thaw How can we share that wake control, gunit save/restore functionality? I am observing issue given below: Here is how I am achieving S0ix: 1. echo mem /sys/power/state 2. (Assuming we have removed all wake_locks in /sys/powe/wake_lock) If device is runtime suspend it is getting resumed with runtime_resume 3. PM core triggers this sequence for each device (prepare, suspend, suspend_late, suspend_noirq) 4. Then pm_suspend happens for Gfx. After all devices are suspended S0i0-S0iR-S0i1-S0i2-S0i3 happens. Gunit gets power gated. While resuming back (with user action/power button press) 1. PM core triggers following sequenc for each device: (resume_noirq, resume_early, resume, complete) After call to i915_drm_thaw_early(called through resume_early), during i915_drm_thaw(called through resume) we were seeing following issue during ring initialization. This is happening since wake is not enabled and Gunit registers are not restored(which is done in runtime_resume but that path is not taken here since this is resume from pm_suspend) We have intel_runtime_pm_get/put calls in the suspend/resume paths which should achieve this. Maybe they're not at the right place or not in the right ordering, but they're there. So on platforms with runtime pm support we _do_ call all the relevant runtime pm callbacks from system suspend/resume. -Daniel I am seeing following commit removed get/put calls from suspend resume paths: commit 395a5abbd97d7d76a7a26da52f33daebe279b3b3 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Wed Jun 18 09:52:56 2014 -0700 drm/i915: don't take runtime PM reference around freeze/thaw That ordering of get/put was also incorrect probably. Even though we do rpm_get before freeze after S0i3 we need to explicitely do wake contro/gunit save restore. In the current patch, the rpm handlers are called directly instead of (intel_runtime_pm_get/put). valleyview_runtime_suspend at the end of i915_drm_freeze and valleyview_runtime_resume at the beginning of i915_drm_thaw_early. Do I need to replace them with intel_display_power_get/put? Will DPM core allow rpm calls when system pm suspend/resume is in progress? No, you can't make the device runtime suspend while the system suspend handler runs since DPM takes an RPM reference before calling this handler. The reference will be
Re: [Intel-gfx] 3.16.0: [drm:i915_gem_init] *ERROR* Failed to initialize GPU, declaring it wedged
Daniel Vetter schreef op vr 08-08-2014 om 11:47 [+0200]: Re-adding mailing lists, please don't drop ccs like that. I sincerely apologize. I seriously have no idea why I assumed a dance for two. On Fri, Aug 8, 2014 at 11:45 AM, Luuk van der Duim luukvanderd...@gmail.com wrote: I'll be happy to test your duct-tape. ;) What branch do I need to pull? (what is its git location address?) git://anongit.freedesktop.org/drm-intel drm-intel-fixes Cheers, Daniel Applied it to a fresh clone of Linus' current git-tree. (the bisecting seems to have altered the tree. git bisect reset did not return a clean tree(?). I have yet to understand most of gits magic.) luuk@Uruburu2:~$ uname -a Linux Uruburu2 3.16.0-drm-fixes-08720-gbd68988 #1 SMP PREEMPT Fri Aug 8 15:10:52 CEST 2014 x86_64 GNU/Linux luuk@Uruburu2 luuk@Uruburu2:~$ dmesg | grep i915 [0.621044] i915 :00:02.0: irq 45 for MSI/MSI-X [0.730160] i915 :00:02.0: fb0: inteldrmfb frame buffer device [0.730161] i915 :00:02.0: registered panic notifier [0.733062] [drm] Initialized i915 1.6.0 20140725 for :00:02.0 on minor 0 [ 75.618392] chrome[3488]: segfault at 8 ip 7f16b9a05e73 sp 7fff0b6951e0 error 4 in i915_dri.so[7f16b9772000+4fd000] As you can see, initialization went fine and, as a consequence, the desktop zoom works fine too. Good! The segfault of a chrome, I don't know, I am using xorg-video-intel driver from the the experimental repository: 2.99.914-1 Thanks! Luuk ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/2] igt/gem_workarounds: igt to test workaround registers
On Fri, Aug 08, 2014 at 10:54:56AM +0100, arun.siluv...@linux.intel.com wrote:
From: Arun Siluvery arun.siluv...@linux.intel.com
Some of the workarounds are lost followed by a gpu reset, suspend/resume;
this patch adds a test which captures register state before and after
the test scenario.
This test currently verifies only bdw workarounds.
Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
Some comments below.
---
lib/intel_reg.h | 8 ++
tests/Makefile.sources | 1 +
tests/gem_workarounds.c | 211
3 files changed, 220 insertions(+)
create mode 100644 tests/gem_workarounds.c
diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index 86175bb..d015c36 100644
--- a/lib/intel_reg.h
+++ b/lib/intel_reg.h
@@ -3628,4 +3628,12 @@ typedef enum {
#define GEN6_WIZ_HASHING_16x4 GEN6_WIZ_HASHING(1, 0)
#define GEN6_WIZ_HASHING_MASK (GEN6_WIZ_HASHING(1, 1)
16)
+#define GAMTARBMODE 0x04a08
+#define _3D_CHICKEN3 0x02090
+#define GAM_ECOCHK 0x4090
+#define CHICKEN_PAR1_1 0x42080
+#define GEN7_FF_THREAD_MODE 0x20a0
+#define GEN6_RC_SLEEP_PSMI_CONTROL 0x2050
+#define GEN8_UCGCTL6 0x9430
+
#endif /* _I810_REG_H */
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 0eb9369..a17acd1 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -134,6 +134,7 @@ TESTS_progs = \
gem_unfence_active_buffers \
gem_unref_active_buffers \
gem_wait_render_timeout \
+ gem_workarounds \
gen3_mixed_blits \
gen3_render_linear_blits \
gen3_render_mixed_blits \
diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
new file mode 100644
index 000..35d1aa7
--- /dev/null
+++ b/tests/gem_workarounds.c
@@ -0,0 +1,211 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ * Arun Siluvery arun.siluv...@linux.intel.com
+ *
+ */
+
+#define _GNU_SOURCE
+#include stdbool.h
+#include unistd.h
+#include stdlib.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include sys/stat.h
+#include sys/ioctl.h
+#include sys/mman.h
+#include time.h
+#include signal.h
+
+#include ioctl_wrappers.h
+#include drmtest.h
+#include igt_debugfs.h
+#include igt_aux.h
+#include intel_chipset.h
+#include intel_io.h
+
+int drm_fd;
+static drm_intel_bufmgr *bufmgr;
+struct intel_batchbuffer *batch;
+uint32_t devid;
+
+enum operation {
+ GPU_RESET,
+ SUSPEND_RESUME,
The suspend test doesn't seem to be wire up ...
Also I think it would be worth to have a module-reload version here too.
+};
+
+struct workaround {
+ const char *reg_name;
+ uint32_t address;
+};
+
+static struct workaround bdw_workarounds[] =
+{
+ { GEN8_ROW_CHICKEN, GEN8_ROW_CHICKEN },
+ { GEN7_ROW_CHICKEN2, GEN7_ROW_CHICKEN2 },
+ { HALF_SLICE_CHICKEN3, HALF_SLICE_CHICKEN3 },
+ { GEN7_HALF_SLICE_CHICKEN1, GEN7_HALF_SLICE_CHICKEN1 },
+ { COMMON_SLICE_CHICKEN2, COMMON_SLICE_CHICKEN2 },
+ { HDC_CHICKEN0, HDC_CHICKEN0 },
+ { GEN7_CACHE_MODE_1, GEN7_CACHE_MODE_1 },
+ { GEN7_GT_MODE, GEN7_GT_MODE },
+ { GAMTARBMODE, GAMTARBMODE },
+ { _3D_CHICKEN3, _3D_CHICKEN3 },
+ { GAM_ECOCHK, GAM_ECOCHK },
+ { CHICKEN_PAR1_1, CHICKEN_PAR1_1 },
+ { GEN7_FF_THREAD_MODE, GEN7_FF_THREAD_MODE },
+ { GEN6_RC_SLEEP_PSMI_CONTROL, GEN6_RC_SLEEP_PSMI_CONTROL },
+ { GEN8_UCGCTL6, GEN8_UCGCTL6 },
+ { NULL, 0x },
+};
Crazy idea I've just had to validate that all the w/a table here is
up-to-date with the one in the kernel:
- We create a special WA_REG macro in the kernel
Re: [Intel-gfx] [PATCH] drm/i915: Continuation of future readiness series
On Fri, Aug 08, 2014 at 02:55:10PM +0300, Ville Syrjälä wrote:
On Fri, Aug 08, 2014 at 05:09:14PM +0530, sonika.jin...@intel.com wrote:
From: Sonika Jindal sonika.jin...@intel.com
Removing the check for HAS_PCH_SPLIT, it looks redundant here. Anyways all
the
platforms are checked separately.
Signed-off-by: Sonika Jindal sonika.jin...@intel.com
---
drivers/gpu/drm/i915/intel_display.c | 40
--
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 89e0ac5..5a3e239 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12421,27 +12421,25 @@ static void intel_init_display(struct drm_device
*dev)
dev_priv-display.get_display_clock_speed =
i830_get_display_clock_speed;
- if (HAS_PCH_SPLIT(dev)) {
- if (IS_GEN5(dev)) {
- dev_priv-display.fdi_link_train =
ironlake_fdi_link_train;
- dev_priv-display.write_eld = ironlake_write_eld;
- } else if (IS_GEN6(dev)) {
- dev_priv-display.fdi_link_train = gen6_fdi_link_train;
- dev_priv-display.write_eld = ironlake_write_eld;
- dev_priv-display.modeset_global_resources =
- snb_modeset_global_resources;
- } else if (IS_IVYBRIDGE(dev)) {
- /* FIXME: detect B0+ stepping and use auto training */
- dev_priv-display.fdi_link_train =
ivb_manual_fdi_link_train;
- dev_priv-display.write_eld = ironlake_write_eld;
- dev_priv-display.modeset_global_resources =
- ivb_modeset_global_resources;
- } else if (IS_HASWELL(dev) || IS_GEN8(dev)) {
- dev_priv-display.fdi_link_train = hsw_fdi_link_train;
- dev_priv-display.write_eld = haswell_write_eld;
- dev_priv-display.modeset_global_resources =
- haswell_modeset_global_resources;
- }
+ if (IS_GEN5(dev)) {
+ dev_priv-display.fdi_link_train = ironlake_fdi_link_train;
+ dev_priv-display.write_eld = ironlake_write_eld;
+ } else if (IS_GEN6(dev)) {
+ dev_priv-display.fdi_link_train = gen6_fdi_link_train;
+ dev_priv-display.write_eld = ironlake_write_eld;
+ dev_priv-display.modeset_global_resources =
+ snb_modeset_global_resources;
+ } else if (IS_IVYBRIDGE(dev)) {
+ /* FIXME: detect B0+ stepping and use auto training */
+ dev_priv-display.fdi_link_train = ivb_manual_fdi_link_train;
+ dev_priv-display.write_eld = ironlake_write_eld;
+ dev_priv-display.modeset_global_resources =
+ ivb_modeset_global_resources;
+ } else if (IS_HASWELL(dev) || IS_GEN8(dev)) {
+ dev_priv-display.fdi_link_train = hsw_fdi_link_train;
+ dev_priv-display.write_eld = haswell_write_eld;
+ dev_priv-display.modeset_global_resources =
+ haswell_modeset_global_resources;
Maybe shuffle these around a bit while you're at it so that the checks
would be roughly in gen order.
Agreed, that might be useful while we touch the code. There's no
preference over newer-first or older-first ordering, so you can pick what
you like. G4X = gen4.5, so oldest platform that supports DP.
-Daniel
} else if (IS_G4X(dev)) {
dev_priv-display.write_eld = g4x_write_eld;
} else if (IS_VALLEYVIEW(dev)) {
--
1.7.10.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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
+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] 3.16.0: [drm:i915_gem_init] *ERROR* Failed to initialize GPU, declaring it wedged
On Fri, Aug 08, 2014 at 04:09:30PM +0200, Luuk van der Duim wrote: Daniel Vetter schreef op vr 08-08-2014 om 11:47 [+0200]: Re-adding mailing lists, please don't drop ccs like that. I sincerely apologize. I seriously have no idea why I assumed a dance for two. On Fri, Aug 8, 2014 at 11:45 AM, Luuk van der Duim luukvanderd...@gmail.com wrote: I'll be happy to test your duct-tape. ;) What branch do I need to pull? (what is its git location address?) git://anongit.freedesktop.org/drm-intel drm-intel-fixes Cheers, Daniel Applied it to a fresh clone of Linus' current git-tree. (the bisecting seems to have altered the tree. git bisect reset did not return a clean tree(?). I have yet to understand most of gits magic.) luuk@Uruburu2:~$ uname -a Linux Uruburu2 3.16.0-drm-fixes-08720-gbd68988 #1 SMP PREEMPT Fri Aug 8 15:10:52 CEST 2014 x86_64 GNU/Linux luuk@Uruburu2 luuk@Uruburu2:~$ dmesg | grep i915 [0.621044] i915 :00:02.0: irq 45 for MSI/MSI-X [0.730160] i915 :00:02.0: fb0: inteldrmfb frame buffer device [0.730161] i915 :00:02.0: registered panic notifier [0.733062] [drm] Initialized i915 1.6.0 20140725 for :00:02.0 on minor 0 [ 75.618392] chrome[3488]: segfault at 8 ip 7f16b9a05e73 sp 7fff0b6951e0 error 4 in i915_dri.so[7f16b9772000+4fd000] As you can see, initialization went fine and, as a consequence, the desktop zoom works fine too. Good! The segfault of a chrome, I don't know, I am using xorg-video-intel driver from the the experimental repository: 2.99.914-1 It's i915_dri.so from mesa that is broken. It is trying to use DRI3 and dies. But the easiest way is probably just to disable DRI3 with Section Device Identifier intel Option DRI 2 EndSection in xorg.conf (or /etc/X11/xorg.conf.d/intel.conf) -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] [PULL] drm-intel-fixes
Hi Dave, So I heard that proper pull requests have a revert on top ;-) So here we go with my usual mid-merge-window pile of fixes. Big fix is the duct-tape for ring init on g4x platforms, we seem to have found the magic again to make those machines as happy as before (not perfect though unfortunately, but that was never the case). Otherwise fixes all over: - tune down some overzealous debug output - VDD power sequencing fix after resume - bunch of dsi fixes for baytrail among them hw state checker de-noising - bunch of error state capture fixes for bdw - misc tiny fixes/workarounds for various platforms Last minute rebase was to kick out two patches that shouldn't have been in here - they're for the state checker, so 0 functional code affected. Jani's back from vacation, so he'll take over -fixes from here. Cc'ing Linus since you're travelling in case you want him to pick this up directly. Cheers, Daniel The following changes since commit a91576d7916f6cce76d30303e60e1ac47cf4a76d: drm/ttm: Pass GFP flags in order to avoid deadlock. (2014-08-05 10:54:19 +1000) are available in the git repository at: git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2014-08-08 for you to fetch changes up to be71eabebaf9f142612d34d42292b454e984dcb5: Revert drm/i915: Enable semaphores on BDW (2014-08-08 16:22:19 +0200) Daniel Vetter (4): drm/i915: Update DRIVER_DATE to 20140725 drm/i915: Don't require dev-struct_mutex in psr_match_conditions drm/i915: Tune done rc6 enabling output drm/i915: Tune down MCH_SSKPD values warning Deepak S (1): drm/i915: Bring GPU Freq to min while suspending. Imre Deak (2): drm/i915: factor out intel_edp_panel_vdd_sanitize drm/i915: fix VDD state tracking after system resume Jiri Kosina (1): drm/i915: read HEAD register back in init_ring_common() to enforce ordering Kenneth Graunke (2): drm/i915: Refactor Broadwell PIPE_CONTROL emission into a helper. drm/i915: Add the WaCsStallBeforeStateCacheInvalidate:bdw workaround. Mika Kuoppala (1): drm/i915: Don't accumulate hangcheck score on forward progress Pavel Machek (1): drm/i915: work around warning in i915_gem_gtt Rafael Barbalho (1): drm/i915: Fix crash when failing to parse MIPI VBT Rodrigo Vivi (4): drm/i915: Fix error state collecting drm/i915: Collect gtier properly on HSW. drm/i915: Fix DEIER and GTIER collecting for BDW. Revert drm/i915: Enable semaphores on BDW Shobhit Kumar (2): drm/i915: wait for all DSI FIFOs to be empty drm/i915: Add correct hw/sw config check for DSI encoder Ville Syrjälä (1): drm/i915: Fix threshold for choosing 32 vs. 64 precisions for VLV DDL values Zhenyu Wang (1): drm/i915: Fix drain latency precision multipler for VLV drivers/gpu/drm/i915/i915_drv.c | 4 ++ drivers/gpu/drm/i915/i915_drv.h | 3 +- drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 11 +++-- drivers/gpu/drm/i915/i915_gpu_error.c | 35 +- drivers/gpu/drm/i915/i915_irq.c | 15 -- drivers/gpu/drm/i915/i915_reg.h | 50 ++-- drivers/gpu/drm/i915/intel_bios.c | 2 +- drivers/gpu/drm/i915/intel_display.c| 4 ++ drivers/gpu/drm/i915/intel_dp.c | 67 +++ drivers/gpu/drm/i915/intel_drv.h| 1 + drivers/gpu/drm/i915/intel_dsi.c| 29 +++- drivers/gpu/drm/i915/intel_dsi.h| 1 + drivers/gpu/drm/i915/intel_dsi_cmd.c| 16 +++ drivers/gpu/drm/i915/intel_dsi_cmd.h| 1 + drivers/gpu/drm/i915/intel_dsi_pll.c| 81 + drivers/gpu/drm/i915/intel_pm.c | 41 - drivers/gpu/drm/i915/intel_ringbuffer.c | 47 +-- drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + 19 files changed, 310 insertions(+), 102 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] Are there any patches addressing Haswell GPU hang 80229
https://bugs.freedesktop.org/show_bug.cgi?id=80229 We can reproduce this GPU hang here after an hours or so of testing. If there are anything I can do to help debug this please let me know. Barry ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] PCI: Add another ID for Intel GPU spurious interrupt quirk
New Intel G3258 CPU, new MSI board, same problem: The GPU interrupt fired like crazy on monitor unplug. lspci output: 00:02.0 VGA compatible controller: Intel Corporation Device 0402 (rev 06) Subsystem: Micro-Star International Co., Ltd. Device 7817 Flags: bus master, fast devsel, latency 0, IRQ 11 Signed-off-by: Thomas Jarosch thomas.jaro...@intra2net.com Tested-by: Thomas Jarosch thomas.jaro...@intra2net.com CC: sta...@vger.kernel.org # v3.4+ --- drivers/pci/quirks.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 80c2d01..d2ff39d 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -2940,6 +2940,7 @@ static void disable_igfx_irq(struct pci_dev *dev) DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0102, disable_igfx_irq); DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x010a, disable_igfx_irq); DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0152, disable_igfx_irq); +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0402, disable_igfx_irq); /* * PCI devices which are on Intel chips can skip the 10ms delay -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] PCI: Add another ID for Intel GPU spurious interrupt quirk
On Fri, Aug 08, 2014 at 03:54:04PM +0200, Thomas Jarosch wrote: New Intel G3258 CPU, new MSI board, same problem: The GPU interrupt fired like crazy on monitor unplug. lspci output: 00:02.0 VGA compatible controller: Intel Corporation Device 0402 (rev 06) Subsystem: Micro-Star International Co., Ltd. Device 7817 Flags: bus master, fast devsel, latency 0, IRQ 11 Signed-off-by: Thomas Jarosch thomas.jaro...@intra2net.com Tested-by: Thomas Jarosch thomas.jaro...@intra2net.com CC: sta...@vger.kernel.org # v3.4+ --- drivers/pci/quirks.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 80c2d01..d2ff39d 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -2940,6 +2940,7 @@ static void disable_igfx_irq(struct pci_dev *dev) DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0102, disable_igfx_irq); DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x010a, disable_igfx_irq); DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0152, disable_igfx_irq); +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0402, disable_igfx_irq); Shouldn't we just add entries for them all? See include/drm/i915_pciids.h We might need to abstract the macro magic a bit though to reuse that list. Also note that not all have the DEIER register. In any case I don't think this game of whack-a-mole here is, we should plug this all for real. -Daniel /* * PCI devices which are on Intel chips can skip the 10ms delay -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: No busy-loop wait_for in the ring init code
On Thu, Aug 07, 2014 at 03:15:32PM +0100, Chris Wilson wrote:
On Thu, Aug 07, 2014 at 04:07:59PM +0200, Daniel Vetter wrote:
Doing a 1s wait (tops) with the cpu is a bit excessive. Tune it down
like everything else in that code.
Cc: Naresh Kumar Kachhi naresh.kumar.kac...@intel.com
Cc: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 05969f03c0c1..966d8f72da45 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -476,7 +476,7 @@ static bool stop_ring(struct intel_engine_cs *ring)
if (!IS_GEN2(ring-dev)) {
I915_WRITE_MODE(ring, _MASKED_BIT_ENABLE(STOP_RING));
- if (wait_for_atomic((I915_READ_MODE(ring) MODE_IDLE) != 0,
1000)) {
+ if (wait_for((I915_READ_MODE(ring) MODE_IDLE) != 0, 1000)) {
DRM_ERROR(%s :timed out trying to stop ring\n,
ring-name);
Please fix the %s :timed here as well.
Fixed.
Ok, it seems like I only thought that wait_for_atomic was microseconds,
but wait_for_atomic_us() was a whole seperate interface.
And assumed that counts for an ack.
-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 39/43] drm/i915/bdw: Print context state in debugfs
On Thu, Aug 07, 2014 at 01:24:26PM +0100, Thomas Daniel wrote:
From: Ben Widawsky b...@bwidawsk.net
This has turned out to be really handy in debug so far.
Update:
Since writing this patch, I've gotten similar code upstream for error
state. I've used it quite a bit in debugfs however, and I'd like to keep
it here at least until preemption is working.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
This patch was accidentally dropped in the first Execlists version, and
it has been very useful indeed. Put it back again, but as a standalone
debugfs file.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
v2: Take the device struct_mutex rather than mode_config mutex for
atomic state capture.
Signed-off-by: Thomas Daniel thomas.dan...@intel.com
Reviewed-by: Damien Lespiau damien.lesp...@intel.com
--
Damien
---
drivers/gpu/drm/i915/i915_debugfs.c | 52
+++
1 file changed, 52 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index aca5ff1..a3c958c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1695,6 +1695,57 @@ static int i915_context_status(struct seq_file *m,
void *unused)
return 0;
}
+static int i915_dump_lrc(struct seq_file *m, void *unused)
+{
+ struct drm_info_node *node = (struct drm_info_node *) m-private;
+ struct drm_device *dev = node-minor-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_engine_cs *ring;
+ struct intel_context *ctx;
+ int ret, i;
+
+ if (!i915.enable_execlists) {
+ seq_printf(m, Logical Ring Contexts are disabled\n);
+ return 0;
+ }
+
+ ret = mutex_lock_interruptible(dev-struct_mutex);
+ if (ret)
+ return ret;
+
+ list_for_each_entry(ctx, dev_priv-context_list, link) {
+ for_each_ring(ring, dev_priv, i) {
+ struct drm_i915_gem_object *ctx_obj =
ctx-engine[i].state;
+
+ if (ring-default_context == ctx)
+ continue;
+
+ if (ctx_obj) {
+ struct page *page =
i915_gem_object_get_page(ctx_obj, 1);
+ uint32_t *reg_state = kmap_atomic(page);
+ int j;
+
+ seq_printf(m, CONTEXT: %s %u\n, ring-name,
+
intel_execlists_ctx_id(ctx_obj));
+
+ for (j = 0; j 0x600 / sizeof(u32) / 4; j +=
4) {
+ seq_printf(m, \t[0x%08lx] 0x%08x
0x%08x 0x%08x 0x%08x\n,
+ i915_gem_obj_ggtt_offset(ctx_obj) +
4096 + (j * 4),
+ reg_state[j], reg_state[j + 1],
+ reg_state[j + 2], reg_state[j + 3]);
+ }
+ kunmap_atomic(reg_state);
+
+ seq_putc(m, '\n');
+ }
+ }
+ }
+
+ mutex_unlock(dev-struct_mutex);
+
+ return 0;
+}
+
static int i915_execlists(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m-private;
@@ -3999,6 +4050,7 @@ static const struct drm_info_list i915_debugfs_list[] =
{
{i915_opregion, i915_opregion, 0},
{i915_gem_framebuffer, i915_gem_framebuffer_info, 0},
{i915_context_status, i915_context_status, 0},
+ {i915_dump_lrc, i915_dump_lrc, 0},
{i915_execlists, i915_execlists, 0},
{i915_gen6_forcewake_count, i915_gen6_forcewake_count_info, 0},
{i915_swizzle_info, i915_swizzle_info, 0},
--
1.7.9.5
___
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 12/43] drm/i915/bdw: Don't write PDP in the legacy way when using LRCs
On Thu, Aug 07, 2014 at 01:17:40PM +0100, Thomas Daniel wrote:
From: Oscar Mateo oscar.ma...@intel.com
This is mostly for correctness so that we know we are running the LR
context correctly (this is, the PDPs are contained inside the context
object).
v2: Move the check to inside the enable PPGTT function. The switch
happens in two places: the legacy context switch (that we won't hit
when Execlists are enabled) and the PPGTT enable, which unfortunately
we need. This would look much nicer if the ppgtt-enable was part of
the ring init, where it logically belongs.
v3: Move the check to the start of the enable PPGTT function. None
of the legacy PPGTT enabling is required when using LRCs as the
PPGTT is enabled in the context descriptor and the PDPs are written
in the LRC.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
Signed-off-by: Thomas Daniel thomas.dan...@intel.com
Reviewed-by: Damien Lespiau damien.lesp...@intel.com
--
Damien
---
drivers/gpu/drm/i915/i915_gem_gtt.c |5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 5188936..cfbf272 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -843,6 +843,11 @@ static int gen8_ppgtt_enable(struct i915_hw_ppgtt *ppgtt)
struct intel_engine_cs *ring;
int j, ret;
+ /* In the case of Execlists, we don't want to write the PDPs
+ * in the legacy way (they live inside the context now) */
+ if (i915.enable_execlists)
+ return 0;
+
for_each_ring(ring, dev_priv, j) {
I915_WRITE(RING_MODE_GEN7(ring),
_MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
--
1.7.9.5
___
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 37/43] drm/i915/bdw: Display execlists info in debugfs
On Thu, Aug 07, 2014 at 01:23:20PM +0100, Thomas Daniel wrote:
From: Oscar Mateo oscar.ma...@intel.com
v2: Warn and return if LRCs are not enabled.
v3: Grab the Execlists spinlock (noticed by Daniel Vetter).
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
v4: Lock the struct mutex for atomic state capture
Signed-off-by: Thomas Daniel thomas.dan...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 80
+++
drivers/gpu/drm/i915/intel_lrc.c|6 ---
drivers/gpu/drm/i915/intel_lrc.h|7 +++
3 files changed, 87 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index fc39610..f8f0e11 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1674,6 +1674,85 @@ static int i915_context_status(struct seq_file *m,
void *unused)
return 0;
}
+static int i915_execlists(struct seq_file *m, void *data)
+{
+ struct drm_info_node *node = (struct drm_info_node *) m-private;
+ struct drm_device *dev = node-minor-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_engine_cs *ring;
+ u32 status_pointer;
+ u8 read_pointer;
+ u8 write_pointer;
+ u32 status;
+ u32 ctx_id;
+ struct list_head *cursor;
+ int ring_id, i;
+ int ret;
+
+ if (!i915.enable_execlists) {
+ seq_printf(m, Logical Ring Contexts are disabled\n);
+ return 0;
+ }
checkpatch.pl will tell you seq_puts() should be used here I guess.
Reviewed-by: Damien Lespiau damien.lesp...@intel.com
--
Damien
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC 2/2] igt/gem_workarounds: igt to test workaround registers
On 08/08/2014 15:12, Daniel Vetter wrote:
On Fri, Aug 08, 2014 at 10:54:56AM +0100, arun.siluv...@linux.intel.com wrote:
From: Arun Siluvery arun.siluv...@linux.intel.com
Some of the workarounds are lost followed by a gpu reset, suspend/resume;
this patch adds a test which captures register state before and after
the test scenario.
This test currently verifies only bdw workarounds.
Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
Some comments below.
---
lib/intel_reg.h | 8 ++
tests/Makefile.sources | 1 +
tests/gem_workarounds.c | 211
3 files changed, 220 insertions(+)
create mode 100644 tests/gem_workarounds.c
diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index 86175bb..d015c36 100644
--- a/lib/intel_reg.h
+++ b/lib/intel_reg.h
@@ -3628,4 +3628,12 @@ typedef enum {
#define GEN6_WIZ_HASHING_16x4 GEN6_WIZ_HASHING(1, 0)
#define GEN6_WIZ_HASHING_MASK (GEN6_WIZ_HASHING(1, 1)
16)
+#define GAMTARBMODE0x04a08
+#define _3D_CHICKEN3 0x02090
+#define GAM_ECOCHK 0x4090
+#define CHICKEN_PAR1_1 0x42080
+#define GEN7_FF_THREAD_MODE0x20a0
+#define GEN6_RC_SLEEP_PSMI_CONTROL 0x2050
+#define GEN8_UCGCTL6 0x9430
+
#endif /* _I810_REG_H */
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 0eb9369..a17acd1 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -134,6 +134,7 @@ TESTS_progs = \
gem_unfence_active_buffers \
gem_unref_active_buffers \
gem_wait_render_timeout \
+ gem_workarounds \
gen3_mixed_blits \
gen3_render_linear_blits \
gen3_render_mixed_blits \
diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c
new file mode 100644
index 000..35d1aa7
--- /dev/null
+++ b/tests/gem_workarounds.c
@@ -0,0 +1,211 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the Software),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ * Arun Siluvery arun.siluv...@linux.intel.com
+ *
+ */
+
+#define _GNU_SOURCE
+#include stdbool.h
+#include unistd.h
+#include stdlib.h
+#include stdio.h
+#include string.h
+#include fcntl.h
+#include inttypes.h
+#include errno.h
+#include sys/stat.h
+#include sys/ioctl.h
+#include sys/mman.h
+#include time.h
+#include signal.h
+
+#include ioctl_wrappers.h
+#include drmtest.h
+#include igt_debugfs.h
+#include igt_aux.h
+#include intel_chipset.h
+#include intel_io.h
+
+int drm_fd;
+static drm_intel_bufmgr *bufmgr;
+struct intel_batchbuffer *batch;
+uint32_t devid;
+
+enum operation {
+ GPU_RESET,
+ SUSPEND_RESUME,
The suspend test doesn't seem to be wire up ...
Also I think it would be worth to have a module-reload version here too.
Suspend/Resume is not working; device is not resuming even after the
timer is elapsed. Do we know suspend/resume works correctly on nightly?
+};
+
+struct workaround {
+ const char *reg_name;
+ uint32_t address;
+};
+
+static struct workaround bdw_workarounds[] =
+{
+ { GEN8_ROW_CHICKEN, GEN8_ROW_CHICKEN },
+ { GEN7_ROW_CHICKEN2, GEN7_ROW_CHICKEN2 },
+ { HALF_SLICE_CHICKEN3, HALF_SLICE_CHICKEN3 },
+ { GEN7_HALF_SLICE_CHICKEN1, GEN7_HALF_SLICE_CHICKEN1 },
+ { COMMON_SLICE_CHICKEN2, COMMON_SLICE_CHICKEN2 },
+ { HDC_CHICKEN0, HDC_CHICKEN0 },
+ { GEN7_CACHE_MODE_1, GEN7_CACHE_MODE_1 },
+ { GEN7_GT_MODE, GEN7_GT_MODE },
+ { GAMTARBMODE, GAMTARBMODE },
+ { _3D_CHICKEN3, _3D_CHICKEN3 },
+ { GAM_ECOCHK, GAM_ECOCHK },
+ { CHICKEN_PAR1_1, CHICKEN_PAR1_1 },
+ { GEN7_FF_THREAD_MODE, GEN7_FF_THREAD_MODE },
+ { GEN6_RC_SLEEP_PSMI_CONTROL, GEN6_RC_SLEEP_PSMI_CONTROL },
+ { GEN8_UCGCTL6, GEN8_UCGCTL6 },
+ { NULL, 0x },
+};
Crazy idea I've just had to
Re: [Intel-gfx] [RFC 2/2] igt/gem_workarounds: igt to test workaround registers
On Fri, Aug 8, 2014 at 6:39 PM, Siluvery, Arun
arun.siluv...@linux.intel.com wrote:
The suspend test doesn't seem to be wire up ...
Also I think it would be worth to have a module-reload version here too.
Suspend/Resume is not working; device is not resuming even after the timer
is elapsed. Do we know suspend/resume works correctly on nightly?
Should still work. You can run one of the basic tests in drv_suspend
to check, or run the underlying cmd used by the igt helper library.
Could very well be that the bios on your machine doesn't support
autoresume for some reasons, so manually waking up is useful to test.
+};
+
+struct workaround {
+ const char *reg_name;
+ uint32_t address;
+};
+
+static struct workaround bdw_workarounds[] =
+{
+ { GEN8_ROW_CHICKEN, GEN8_ROW_CHICKEN },
+ { GEN7_ROW_CHICKEN2, GEN7_ROW_CHICKEN2 },
+ { HALF_SLICE_CHICKEN3, HALF_SLICE_CHICKEN3 },
+ { GEN7_HALF_SLICE_CHICKEN1, GEN7_HALF_SLICE_CHICKEN1 },
+ { COMMON_SLICE_CHICKEN2, COMMON_SLICE_CHICKEN2 },
+ { HDC_CHICKEN0, HDC_CHICKEN0 },
+ { GEN7_CACHE_MODE_1, GEN7_CACHE_MODE_1 },
+ { GEN7_GT_MODE, GEN7_GT_MODE },
+ { GAMTARBMODE, GAMTARBMODE },
+ { _3D_CHICKEN3, _3D_CHICKEN3 },
+ { GAM_ECOCHK, GAM_ECOCHK },
+ { CHICKEN_PAR1_1, CHICKEN_PAR1_1 },
+ { GEN7_FF_THREAD_MODE, GEN7_FF_THREAD_MODE },
+ { GEN6_RC_SLEEP_PSMI_CONTROL, GEN6_RC_SLEEP_PSMI_CONTROL },
+ { GEN8_UCGCTL6, GEN8_UCGCTL6 },
+ { NULL, 0x },
+};
Crazy idea I've just had to validate that all the w/a table here is
up-to-date with the one in the kernel:
- We create a special WA_REG macro in the kernel which we use to wrap all
registers used in workarounds at the specific use-site (i.e. not in the
header). So
I951_WRITE(WA_REG(GEN8_ROW_CHICKEN), );
- That macro then adds the register to a table which we can dump through
debugs with a file called intel_wa_registers. This happens at runtime.
This is important since a static list over all platforms might included
registers which hang some platforms when we read them.
- A special subtest in this test here compares the kernel-provided list
with the one supplied here and makes sure that all the w/a in the
kernel
list are in the test list, too. Or we just ditch the test list here
completely, but that might not work for special cases where we only
need
to check some masks ...
Opinions on this? Would this help with maintaining this testcase and
ensuring that it is always up-to-date with the kernel w/a list? I really
want to make sure we get this right, there's been way too many cases where
w/a settings have been lost over resume, runtime pm, ctx switches ...
I will change the implementation to use this macro.
so in this case the table is updated before each use case (reset,
suspend/resume, module reload etc)? Is it not sufficient to capture the
state at the beginning? my understanding is the wa state should really stay
the same and we compare the current state (eg after reset) to the one at the
beginning rather than the state before reset.
Yeah, just grabbing the wa register list once from debugfs in an
igt_fixture block is good enough.
I think it is easier to maintain if we completely remove the workaround list
from igt itself, based on hardware macro can populate only those workarounds
that are applicable but you mentioned that may not work for special cases,
could you elaborate about these cases?
Some w/a are set in registers which have other stuff that changes at
runtime, iirc some of the ring-related registers are set in RING_CTL.
So for those we'd need a mask to make sure we exclude the bits that
vary. But we can solve that problem once we have it, your current list
doesn't have any such special case.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix erroneous conversion to u8
adj was defined as u8. The issue is last_adj can be negative and adj is
initialized with:
adj = dev_priv-rps.last_adj;
and we were also happily doing things like:
if (adj 0)
(thank static analysers!)
Cc: Deepak S deepa...@linux.intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9fdf738..89e633f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1330,7 +1330,8 @@ static u32 vlv_c0_residency(struct drm_i915_private
*dev_priv,
static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
{
u32 residency_C0_up = 0, residency_C0_down = 0;
- u8 new_delay, adj;
+ u8 new_delay;
+ int adj;
dev_priv-rps.ei_interrupt_count++;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix erroneous conversion to u8
On Fri, Aug 08, 2014 at 06:34:48PM +0100, Damien Lespiau wrote:
adj was defined as u8. The issue is last_adj can be negative and adj is
initialized with:
adj = dev_priv-rps.last_adj;
and we were also happily doing things like:
if (adj 0)
(thank static analysers!)
Cc: Deepak S deepa...@linux.intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9fdf738..89e633f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1330,7 +1330,8 @@ static u32 vlv_c0_residency(struct drm_i915_private
*dev_priv,
static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
{
u32 residency_C0_up = 0, residency_C0_down = 0;
- u8 new_delay, adj;
+ u8 new_delay;
+ int adj;
Might be better to make new_delay int too, so that we don't accidentally
under/overflow it due to the adj acceleration thing. And the return
value too. I'm feeling too lazy to think what kind of conditions that
would required, so just making it all int seems easier.
dev_priv-rps.ei_interrupt_count++;
--
1.8.3.1
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix erroneous conversion to u8
adj was defined as u8. The issue is last_adj can be negative and adj is
initialized with:
adj = dev_priv-rps.last_adj;
and we were also happily doing things like:
if (adj 0)
(thank static analysers!)
v2: Make new_delay an int in case we overflow the u8 in the intermediate
computations. new_delay will get clamped at the end anyway. (Ville)
Cc: Deepak S deepa...@linux.intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9fdf738..8e6729e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1327,10 +1327,10 @@ static u32 vlv_c0_residency(struct drm_i915_private
*dev_priv,
* @dev_priv: DRM device private
*
*/
-static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
+static int vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
{
u32 residency_C0_up = 0, residency_C0_down = 0;
- u8 new_delay, adj;
+ int new_delay, adj;
dev_priv-rps.ei_interrupt_count++;
--
1.8.3.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Track cursor changes as frontbuffer tracking flushes
We treat other plane updates in the same fashion. Spotted because
Rodrigo kept reporting a bug in the PSR code where the frontbuffer was
eternally stuck with a dirty cursor bit set.
The psr testcase should have caught this, but that i-g-t is kaputt.
Rodrigo is signed up to fix that.
Cc: Rodrigo Vivi rodrigo.v...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index fac7bdfe9ec8..91984710b841 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11677,6 +11677,10 @@ intel_cursor_plane_update(struct drm_plane *plane,
struct drm_crtc *crtc,
return intel_crtc_cursor_set_obj(crtc, obj, crtc_w, crtc_h);
} else {
intel_crtc_update_cursor(crtc, visible);
+
+ intel_frontbuffer_flip(crtc-dev,
+
INTEL_FRONTBUFFER_CURSOR(intel_crtc-pipe));
+
return 0;
}
}
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: static inline for intel_wait_for_vblank
Requested by Chris, and also requested to keep it since it's a
more accurate name in his opinion.
Cc: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 13 -
drivers/gpu/drm/i915/intel_drv.h | 6 +-
2 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 91984710b841..918a947bf4f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -891,19 +891,6 @@ enum transcoder intel_pipe_to_cpu_transcoder(struct
drm_i915_private *dev_priv,
return intel_crtc-config.cpu_transcoder;
}
-/**
- * intel_wait_for_vblank - wait for vblank on a given pipe
- * @dev: drm device
- * @pipe: pipe to wait for
- *
- * Wait for vblank to occur on a given pipe. Needed for various bits of
- * mode setting code.
- */
-void intel_wait_for_vblank(struct drm_device *dev, int pipe)
-{
- drm_wait_one_vblank(dev, pipe);
-}
-
static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
{
struct drm_i915_private *dev_priv = dev-dev_private;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8a475a6909c3..b8ceb4cb7758 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -820,7 +820,11 @@ int intel_get_pipe_from_crtc_id(struct drm_device *dev,
void *data,
struct drm_file *file_priv);
enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
enum pipe pipe);
-void intel_wait_for_vblank(struct drm_device *dev, int pipe);
+static inline void
+intel_wait_for_vblank(struct drm_device *dev, int pipe)
+{
+ drm_wait_one_vblank(dev, pipe);
+}
void intel_wait_for_pipe_off(struct drm_device *dev, int pipe);
int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp);
void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix erroneous conversion to u8
On Fri, Aug 08, 2014 at 07:25:57PM +0100, Damien Lespiau wrote:
adj was defined as u8. The issue is last_adj can be negative and adj is
initialized with:
adj = dev_priv-rps.last_adj;
and we were also happily doing things like:
if (adj 0)
(thank static analysers!)
v2: Make new_delay an int in case we overflow the u8 in the intermediate
computations. new_delay will get clamped at the end anyway. (Ville)
Cc: Deepak S deepa...@linux.intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/i915_irq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9fdf738..8e6729e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1327,10 +1327,10 @@ static u32 vlv_c0_residency(struct drm_i915_private
*dev_priv,
* @dev_priv: DRM device private
*
*/
-static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
+static int vlv_calc_delay_from_C0_counters(struct drm_i915_private *dev_priv)
{
u32 residency_C0_up = 0, residency_C0_down = 0;
- u8 new_delay, adj;
+ int new_delay, adj;
dev_priv-rps.ei_interrupt_count++;
--
1.8.3.1
--
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: static inline for intel_wait_for_vblank
On Fri, Aug 08, 2014 at 08:28:48PM +0200, Daniel Vetter wrote: Requested by Chris, and also requested to keep it since it's a more accurate name in his opinion. Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch 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
[Intel-gfx] Updated drm-intel-testing
Hi all, First -testing cycle for 3.18 already: - Setting dp M2/N2 values plus state checker support (Vandana Kannan) - chv power well support (Ville) - DP training pattern 3 support for chv (Ville) - cleanup of the hsw/bdw ddi pll code, prep work for skl (Damien) - dsi video burst mode support (Shobhit) - piles of other chv fixes all over (Ville et. al.) - cleanup of the ddi translation tables setup code (Damien) - 180 deg rotation support (Ville Sonika Jindal) Happy testing! Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Eliminate rmw from .update_primary_plane()
From: Ville Syrjälä ville.syrj...@linux.intel.com
Move the entire DSPCNTR register setup into the .update_primary_plane()
functions. That's where it belongs anyway and it'll also help 830M which
has the extra problem that plane registers reads will return the value
latched at the last vblank, not the value that was last written.
Also move DSPPOS and DSPSIZE setup there.
v2: Don't move variable initialization to avoid churn later
Reviewed-by: Matt Roper matthew.d.ro...@intel.com
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 100 +++
1 file changed, 32 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 89e0ac5..4158257 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2394,12 +2394,26 @@ static void i9xx_update_primary_plane(struct drm_crtc
*crtc,
int plane = intel_crtc-plane;
unsigned long linear_offset;
u32 dspcntr;
- u32 reg;
+ u32 reg = DSPCNTR(plane);
+
+ dspcntr = DISPPLANE_GAMMA_ENABLE;
+
+ if (intel_crtc-primary_enabled)
+ dspcntr |= DISPLAY_PLANE_ENABLE;
+
+ if (INTEL_INFO(dev)-gen 4) {
+ if (intel_crtc-pipe == PIPE_B)
+ dspcntr |= DISPPLANE_SEL_PIPE_B;
+
+ /* pipesrc and dspsize control the size that is scaled from,
+* which should always be the user's requested size.
+*/
+ I915_WRITE(DSPSIZE(plane),
+ ((intel_crtc-config.pipe_src_h - 1) 16) |
+ (intel_crtc-config.pipe_src_w - 1));
+ I915_WRITE(DSPPOS(plane), 0);
+ }
- reg = DSPCNTR(plane);
- dspcntr = I915_READ(reg);
- /* Mask out pixel format bits in case we change it */
- dspcntr = ~DISPPLANE_PIXFORMAT_MASK;
switch (fb-pixel_format) {
case DRM_FORMAT_C8:
dspcntr |= DISPPLANE_8BPP;
@@ -2431,12 +2445,9 @@ static void i9xx_update_primary_plane(struct drm_crtc
*crtc,
BUG();
}
- if (INTEL_INFO(dev)-gen = 4) {
- if (obj-tiling_mode != I915_TILING_NONE)
- dspcntr |= DISPPLANE_TILED;
- else
- dspcntr = ~DISPPLANE_TILED;
- }
+ if (INTEL_INFO(dev)-gen = 4
+ obj-tiling_mode != I915_TILING_NONE)
+ dspcntr |= DISPPLANE_TILED;
if (IS_G4X(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
@@ -2480,12 +2491,16 @@ static void ironlake_update_primary_plane(struct
drm_crtc *crtc,
int plane = intel_crtc-plane;
unsigned long linear_offset;
u32 dspcntr;
- u32 reg;
+ u32 reg = DSPCNTR(plane);
+
+ dspcntr = DISPPLANE_GAMMA_ENABLE;
+
+ if (intel_crtc-primary_enabled)
+ dspcntr |= DISPLAY_PLANE_ENABLE;
+
+ if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+ dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
- reg = DSPCNTR(plane);
- dspcntr = I915_READ(reg);
- /* Mask out pixel format bits in case we change it */
- dspcntr = ~DISPPLANE_PIXFORMAT_MASK;
switch (fb-pixel_format) {
case DRM_FORMAT_C8:
dspcntr |= DISPPLANE_8BPP;
@@ -2515,12 +2530,8 @@ static void ironlake_update_primary_plane(struct
drm_crtc *crtc,
if (obj-tiling_mode != I915_TILING_NONE)
dspcntr |= DISPPLANE_TILED;
- else
- dspcntr = ~DISPPLANE_TILED;
- if (IS_HASWELL(dev) || IS_BROADWELL(dev))
- dspcntr = ~DISPPLANE_TRICKLE_FEED_DISABLE;
- else
+ if (!IS_HASWELL(dev) !IS_BROADWELL(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
I915_WRITE(reg, dspcntr);
@@ -3946,7 +3957,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_encoder *encoder;
int pipe = intel_crtc-pipe;
- enum plane plane = intel_crtc-plane;
WARN_ON(!crtc-enabled);
@@ -3968,10 +3978,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
ironlake_set_pipeconf(crtc);
- /* Set up the display plane register */
- I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE);
- POSTING_READ(DSPCNTR(plane));
-
dev_priv-display.update_primary_plane(crtc, crtc-primary-fb,
crtc-x, crtc-y);
@@ -4059,7 +4065,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_encoder *encoder;
int pipe = intel_crtc-pipe;
- enum plane plane = intel_crtc-plane;
WARN_ON(!crtc-enabled);
@@ -4083,10 +4088,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Eliminate rmw from .update_primary_plane()
On Fri, Aug 08, 2014 at 09:51:10PM +0300, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Move the entire DSPCNTR register setup into the .update_primary_plane()
functions. That's where it belongs anyway and it'll also help 830M which
has the extra problem that plane registers reads will return the value
latched at the last vblank, not the value that was last written.
Also move DSPPOS and DSPSIZE setup there.
v2: Don't move variable initialization to avoid churn later
Reviewed-by: Matt Roper matthew.d.ro...@intel.com
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
Queued for -next, thanks for the patch.
-Daniel
---
drivers/gpu/drm/i915/intel_display.c | 100
+++
1 file changed, 32 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 89e0ac5..4158257 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2394,12 +2394,26 @@ static void i9xx_update_primary_plane(struct drm_crtc
*crtc,
int plane = intel_crtc-plane;
unsigned long linear_offset;
u32 dspcntr;
- u32 reg;
+ u32 reg = DSPCNTR(plane);
+
+ dspcntr = DISPPLANE_GAMMA_ENABLE;
+
+ if (intel_crtc-primary_enabled)
+ dspcntr |= DISPLAY_PLANE_ENABLE;
+
+ if (INTEL_INFO(dev)-gen 4) {
+ if (intel_crtc-pipe == PIPE_B)
+ dspcntr |= DISPPLANE_SEL_PIPE_B;
+
+ /* pipesrc and dspsize control the size that is scaled from,
+ * which should always be the user's requested size.
+ */
+ I915_WRITE(DSPSIZE(plane),
+((intel_crtc-config.pipe_src_h - 1) 16) |
+(intel_crtc-config.pipe_src_w - 1));
+ I915_WRITE(DSPPOS(plane), 0);
+ }
- reg = DSPCNTR(plane);
- dspcntr = I915_READ(reg);
- /* Mask out pixel format bits in case we change it */
- dspcntr = ~DISPPLANE_PIXFORMAT_MASK;
switch (fb-pixel_format) {
case DRM_FORMAT_C8:
dspcntr |= DISPPLANE_8BPP;
@@ -2431,12 +2445,9 @@ static void i9xx_update_primary_plane(struct drm_crtc
*crtc,
BUG();
}
- if (INTEL_INFO(dev)-gen = 4) {
- if (obj-tiling_mode != I915_TILING_NONE)
- dspcntr |= DISPPLANE_TILED;
- else
- dspcntr = ~DISPPLANE_TILED;
- }
+ if (INTEL_INFO(dev)-gen = 4
+ obj-tiling_mode != I915_TILING_NONE)
+ dspcntr |= DISPPLANE_TILED;
if (IS_G4X(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
@@ -2480,12 +2491,16 @@ static void ironlake_update_primary_plane(struct
drm_crtc *crtc,
int plane = intel_crtc-plane;
unsigned long linear_offset;
u32 dspcntr;
- u32 reg;
+ u32 reg = DSPCNTR(plane);
+
+ dspcntr = DISPPLANE_GAMMA_ENABLE;
+
+ if (intel_crtc-primary_enabled)
+ dspcntr |= DISPLAY_PLANE_ENABLE;
+
+ if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+ dspcntr |= DISPPLANE_PIPE_CSC_ENABLE;
- reg = DSPCNTR(plane);
- dspcntr = I915_READ(reg);
- /* Mask out pixel format bits in case we change it */
- dspcntr = ~DISPPLANE_PIXFORMAT_MASK;
switch (fb-pixel_format) {
case DRM_FORMAT_C8:
dspcntr |= DISPPLANE_8BPP;
@@ -2515,12 +2530,8 @@ static void ironlake_update_primary_plane(struct
drm_crtc *crtc,
if (obj-tiling_mode != I915_TILING_NONE)
dspcntr |= DISPPLANE_TILED;
- else
- dspcntr = ~DISPPLANE_TILED;
- if (IS_HASWELL(dev) || IS_BROADWELL(dev))
- dspcntr = ~DISPPLANE_TRICKLE_FEED_DISABLE;
- else
+ if (!IS_HASWELL(dev) !IS_BROADWELL(dev))
dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
I915_WRITE(reg, dspcntr);
@@ -3946,7 +3957,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_encoder *encoder;
int pipe = intel_crtc-pipe;
- enum plane plane = intel_crtc-plane;
WARN_ON(!crtc-enabled);
@@ -3968,10 +3978,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
ironlake_set_pipeconf(crtc);
- /* Set up the display plane register */
- I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE);
- POSTING_READ(DSPCNTR(plane));
-
dev_priv-display.update_primary_plane(crtc, crtc-primary-fb,
crtc-x, crtc-y);
@@ -4059,7 +4065,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_encoder *encoder;
int pipe = intel_crtc-pipe;
- enum plane plane = intel_crtc-plane;
[Intel-gfx] [PATCH 2/2] drm/i915: Call .update_primary_plane in intel_{enable, disable}_primary_hw_plane()
From: Ville Syrjälä ville.syrj...@linux.intel.com
Make the intel_{enable,disable}_primary_hw_plane() simply call
.update_primary_plane(), thus eliminating the rmw from these functions
which should help the poor old 830M.
Now we can also remove the .update_primary_plane() from the
.crtc_enable() hooks because we end up calling it via
intel_crtc_enable_planes()-intel_enable_primary_hw_plane().
This also has the nice benefit of making primary planes a bit closer to
the way we handle sprite planes during modesets.
v2: Just write 0 to DSPCNTR and DSPSURF/DSPADDR if the plane is (to be)
disabled. Quicker, and more importantly avoids an oops when fb==NULL
due to BIOS fb takeover failure.
Pimp the commit message a bit (Matt)
v3: Drop useless primary_enabled checks when setting DISPLAY_PLANE_ENABLE
Reviewed-by: Matt Roper matthew.d.ro...@intel.com
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
---
drivers/gpu/drm/i915/intel_display.c | 119 +++
1 file changed, 49 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4158257..ca4f8e6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2088,35 +2088,28 @@ void intel_flush_primary_plane(struct drm_i915_private
*dev_priv,
/**
* intel_enable_primary_hw_plane - enable the primary plane on a given pipe
- * @dev_priv: i915 private structure
- * @plane: plane to enable
- * @pipe: pipe being fed
+ * @plane: plane to be enabled
+ * @crtc: crtc for the plane
*
- * Enable @plane on @pipe, making sure that @pipe is running first.
+ * Enable @plane on @crtc, making sure that the pipe is running first.
*/
-static void intel_enable_primary_hw_plane(struct drm_i915_private *dev_priv,
- enum plane plane, enum pipe pipe)
+static void intel_enable_primary_hw_plane(struct drm_plane *plane,
+ struct drm_crtc *crtc)
{
- struct drm_device *dev = dev_priv-dev;
- struct intel_crtc *intel_crtc =
- to_intel_crtc(dev_priv-pipe_to_crtc_mapping[pipe]);
- int reg;
- u32 val;
+ struct drm_device *dev = plane-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
/* If the pipe isn't enabled, we can't pump pixels and may hang */
- assert_pipe_enabled(dev_priv, pipe);
+ assert_pipe_enabled(dev_priv, intel_crtc-pipe);
if (intel_crtc-primary_enabled)
return;
intel_crtc-primary_enabled = true;
- reg = DSPCNTR(plane);
- val = I915_READ(reg);
- WARN_ON(val DISPLAY_PLANE_ENABLE);
-
- I915_WRITE(reg, val | DISPLAY_PLANE_ENABLE);
- intel_flush_primary_plane(dev_priv, plane);
+ dev_priv-display.update_primary_plane(crtc, plane-fb,
+ crtc-x, crtc-y);
/*
* BDW signals flip done immediately if the plane
@@ -2129,31 +2122,27 @@ static void intel_enable_primary_hw_plane(struct
drm_i915_private *dev_priv,
/**
* intel_disable_primary_hw_plane - disable the primary hardware plane
- * @dev_priv: i915 private structure
- * @plane: plane to disable
- * @pipe: pipe consuming the data
+ * @plane: plane to be disabled
+ * @crtc: crtc for the plane
*
- * Disable @plane; should be an independent operation.
+ * Disable @plane on @crtc, making sure that the pipe is running first.
*/
-static void intel_disable_primary_hw_plane(struct drm_i915_private *dev_priv,
- enum plane plane, enum pipe pipe)
+static void intel_disable_primary_hw_plane(struct drm_plane *plane,
+ struct drm_crtc *crtc)
{
- struct intel_crtc *intel_crtc =
- to_intel_crtc(dev_priv-pipe_to_crtc_mapping[pipe]);
- int reg;
- u32 val;
+ struct drm_device *dev = plane-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+ assert_pipe_enabled(dev_priv, intel_crtc-pipe);
if (!intel_crtc-primary_enabled)
return;
intel_crtc-primary_enabled = false;
- reg = DSPCNTR(plane);
- val = I915_READ(reg);
- WARN_ON((val DISPLAY_PLANE_ENABLE) == 0);
-
- I915_WRITE(reg, val ~DISPLAY_PLANE_ENABLE);
- intel_flush_primary_plane(dev_priv, plane);
+ dev_priv-display.update_primary_plane(crtc, plane-fb,
+ crtc-x, crtc-y);
}
static bool need_vtd_wa(struct drm_device *dev)
@@ -2396,10 +2385,19 @@ static void i9xx_update_primary_plane(struct drm_crtc
*crtc,
u32 dspcntr;
u32 reg = DSPCNTR(plane);
+ if (!intel_crtc-primary_enabled) {
+ I915_WRITE(reg, 0);
+ if
Re: [Intel-gfx] [PATCH] drm/i915: Fix erroneous conversion to u8
On Fri, Aug 08, 2014 at 09:32:20PM +0300, Ville Syrjälä wrote:
On Fri, Aug 08, 2014 at 07:25:57PM +0100, Damien Lespiau wrote:
adj was defined as u8. The issue is last_adj can be negative and adj is
initialized with:
adj = dev_priv-rps.last_adj;
and we were also happily doing things like:
if (adj 0)
(thank static analysers!)
v2: Make new_delay an int in case we overflow the u8 in the intermediate
computations. new_delay will get clamped at the end anyway. (Ville)
Cc: Deepak S deepa...@linux.intel.com
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Damien Lespiau damien.lesp...@intel.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Queued for -next, thanks for the patch.
-Daniel
---
drivers/gpu/drm/i915/i915_irq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 9fdf738..8e6729e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1327,10 +1327,10 @@ static u32 vlv_c0_residency(struct drm_i915_private
*dev_priv,
* @dev_priv: DRM device private
*
*/
-static u32 vlv_calc_delay_from_C0_counters(struct drm_i915_private
*dev_priv)
+static int vlv_calc_delay_from_C0_counters(struct drm_i915_private
*dev_priv)
{
u32 residency_C0_up = 0, residency_C0_down = 0;
- u8 new_delay, adj;
+ int new_delay, adj;
dev_priv-rps.ei_interrupt_count++;
--
1.8.3.1
--
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
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: Call .update_primary_plane in intel_{enable, disable}_primary_hw_plane()
On Fri, Aug 08, 2014 at 09:51:11PM +0300, ville.syrj...@linux.intel.com wrote:
From: Ville Syrjälä ville.syrj...@linux.intel.com
Make the intel_{enable,disable}_primary_hw_plane() simply call
.update_primary_plane(), thus eliminating the rmw from these functions
which should help the poor old 830M.
Now we can also remove the .update_primary_plane() from the
.crtc_enable() hooks because we end up calling it via
intel_crtc_enable_planes()-intel_enable_primary_hw_plane().
This also has the nice benefit of making primary planes a bit closer to
the way we handle sprite planes during modesets.
v2: Just write 0 to DSPCNTR and DSPSURF/DSPADDR if the plane is (to be)
disabled. Quicker, and more importantly avoids an oops when fb==NULL
due to BIOS fb takeover failure.
Pimp the commit message a bit (Matt)
v3: Drop useless primary_enabled checks when setting DISPLAY_PLANE_ENABLE
Reviewed-by: Matt Roper matthew.d.ro...@intel.com
Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
This almost starts to look sane ... Queued for -next, thanks for the patch.
-Daniel
---
drivers/gpu/drm/i915/intel_display.c | 119
+++
1 file changed, 49 insertions(+), 70 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 4158257..ca4f8e6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2088,35 +2088,28 @@ void intel_flush_primary_plane(struct
drm_i915_private *dev_priv,
/**
* intel_enable_primary_hw_plane - enable the primary plane on a given pipe
- * @dev_priv: i915 private structure
- * @plane: plane to enable
- * @pipe: pipe being fed
+ * @plane: plane to be enabled
+ * @crtc: crtc for the plane
*
- * Enable @plane on @pipe, making sure that @pipe is running first.
+ * Enable @plane on @crtc, making sure that the pipe is running first.
*/
-static void intel_enable_primary_hw_plane(struct drm_i915_private *dev_priv,
- enum plane plane, enum pipe pipe)
+static void intel_enable_primary_hw_plane(struct drm_plane *plane,
+ struct drm_crtc *crtc)
{
- struct drm_device *dev = dev_priv-dev;
- struct intel_crtc *intel_crtc =
- to_intel_crtc(dev_priv-pipe_to_crtc_mapping[pipe]);
- int reg;
- u32 val;
+ struct drm_device *dev = plane-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
/* If the pipe isn't enabled, we can't pump pixels and may hang */
- assert_pipe_enabled(dev_priv, pipe);
+ assert_pipe_enabled(dev_priv, intel_crtc-pipe);
if (intel_crtc-primary_enabled)
return;
intel_crtc-primary_enabled = true;
- reg = DSPCNTR(plane);
- val = I915_READ(reg);
- WARN_ON(val DISPLAY_PLANE_ENABLE);
-
- I915_WRITE(reg, val | DISPLAY_PLANE_ENABLE);
- intel_flush_primary_plane(dev_priv, plane);
+ dev_priv-display.update_primary_plane(crtc, plane-fb,
+crtc-x, crtc-y);
/*
* BDW signals flip done immediately if the plane
@@ -2129,31 +2122,27 @@ static void intel_enable_primary_hw_plane(struct
drm_i915_private *dev_priv,
/**
* intel_disable_primary_hw_plane - disable the primary hardware plane
- * @dev_priv: i915 private structure
- * @plane: plane to disable
- * @pipe: pipe consuming the data
+ * @plane: plane to be disabled
+ * @crtc: crtc for the plane
*
- * Disable @plane; should be an independent operation.
+ * Disable @plane on @crtc, making sure that the pipe is running first.
*/
-static void intel_disable_primary_hw_plane(struct drm_i915_private *dev_priv,
-enum plane plane, enum pipe pipe)
+static void intel_disable_primary_hw_plane(struct drm_plane *plane,
+struct drm_crtc *crtc)
{
- struct intel_crtc *intel_crtc =
- to_intel_crtc(dev_priv-pipe_to_crtc_mapping[pipe]);
- int reg;
- u32 val;
+ struct drm_device *dev = plane-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+ assert_pipe_enabled(dev_priv, intel_crtc-pipe);
if (!intel_crtc-primary_enabled)
return;
intel_crtc-primary_enabled = false;
- reg = DSPCNTR(plane);
- val = I915_READ(reg);
- WARN_ON((val DISPLAY_PLANE_ENABLE) == 0);
-
- I915_WRITE(reg, val ~DISPLAY_PLANE_ENABLE);
- intel_flush_primary_plane(dev_priv, plane);
+ dev_priv-display.update_primary_plane(crtc, plane-fb,
+crtc-x, crtc-y);
}
static bool need_vtd_wa(struct drm_device *dev)
@@ -2396,10 +2385,19 @@ static void
Re: [Intel-gfx] [PULL] drm-intel-fixes
On Fri, Aug 8, 2014 at 7:34 AM, Daniel Vetter daniel.vet...@ffwll.ch wrote: git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2014-08-08 Hmm. My laptop no longer resumes properly. Or rather, I suspect it resumes, but the screen is black. I will bisect, and I *will* revert if I find the offending commit. I need that laptop for travel next week. Linus ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PULL] drm-intel-fixes
On Fri, Aug 8, 2014 at 12:19 PM, Linus Torvalds torva...@linux-foundation.org wrote: On Fri, Aug 8, 2014 at 7:34 AM, Daniel Vetter daniel.vet...@ffwll.ch wrote: git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2014-08-08 Hmm. My laptop no longer resumes properly. The problem seems to exist already with just the plain drm pull from Dave. I thought I had tested that, but apparently not. Linus ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PULL] drm-intel-fixes
On Fri, Aug 8, 2014 at 12:37 PM, Linus Torvalds torva...@linux-foundation.org wrote: The problem seems to exist already with just the plain drm pull from Dave. I thought I had tested that, but apparently not. Still busy bisecting (and it's going into the i915 part of Dave's drm pull, so the bisect looks sane so far), but in case some i915 person is trying to think about what it might be, it doesn't seem to be the backlight per se. Sometimes it resumes with the backlight on and something showing on the screen, but no reaction to mouse or keyboard. And in fact, I have now gotten an X lockup twice *without* doing resume/suspend, so it seems like the whole resume/suspend thing is just a way to trigger it, not the fundamental problem. The machine is alive (the printscreen button takes a screenshot, for example - I can tell by the sound), but the graphics side is hung. This is a Sony Vaio Pro 11, so it's bog-standard intel graphics (Haswell ULT). Linus ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: fix i915_interrupt_info on BDW
From: Paulo Zanoni paulo.r.zan...@intel.com
Currently, if the machine is runtime suspended an you read the file,
you will get an Unclaimed register error message.
Testcase: igt/pm_rpm/debugfs-read
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_debugfs.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 0748960..c3bf3d5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -703,6 +703,12 @@ static int i915_interrupt_info(struct seq_file *m, void
*data)
}
for_each_pipe(pipe) {
+ if (!intel_display_power_enabled(dev_priv,
+ POWER_DOMAIN_PIPE(pipe))) {
+ seq_printf(m, Pipe %c power disabled\n,
+ pipe_name(pipe));
+ continue;
+ }
seq_printf(m, Pipe %c IMR:\t%08x\n,
pipe_name(pipe),
I915_READ(GEN8_DE_PIPE_IMR(pipe)));
--
2.0.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PULL] drm-intel-fixes
On Fri, Aug 8, 2014 at 10:30 AM, Linus Torvalds torva...@linux-foundation.org wrote: This is a Sony Vaio Pro 11, so it's bog-standard intel graphics (Haswell ULT). Got this while bisecting. I'm not sure it's related, the behavior was a bit different from the other cases. So I'll try to continue bisecting, but am a bit worried that there are two possibly unrelated problems here.. [ cut here ] WARNING: CPU: 0 PID: 1074 at drivers/gpu/drm/i915/intel_display.c:1234 intel_enable_pipe+0x3f/0x200 [i915]() cursor on pipe A assertion failure (expected off, current on) Modules linked in: rfcomm fuse ccm ip6t_rpfilter ip6t_REJECT xt_conntrack mmc_block ebtable_nat ebtable_broute bridge stp llc ebtable_filter ebtables ip6table_nat n$ snd_pcm media btusb bluetooth hid_multitouch iwlwifi snd_timer rtsx_pci i2c_i801 cfg80211 lpc_ich mfd_core mei_me snd mei shpchp soundcore sony_laptop rfkill dm_cr$ CPU: 0 PID: 1074 Comm: Xorg Not tainted 3.16.0-rc4-00378-g4dac3edfe68e #15 Hardware name: Sony Corporation SVP11213CXB/VAIO, BIOS R0270V7 05/17/2013 Call Trace: ? dump_stack+0x41/0x51 ? warn_slowpath_common+0x6d/0x90 ? warn_slowpath_fmt+0x47/0x50 ? intel_enable_pipe+0x3f/0x200 [i915] ? haswell_crtc_enable+0x425/0xa70 [i915] ? intel_crtc_update_dpms+0x5e/0x70 [i915] ? intel_connector_dpms+0x39/0x70 [i915] ? drm_mode_obj_set_property_ioctl+0x396/0x3b0 [drm] ? drm_mode_connector_property_set_ioctl+0x29/0x30 [drm] ? drm_ioctl+0x178/0x580 [drm] ? do_vfs_ioctl+0x2cf/0x4b0 ? file_has_perm+0x86/0xa0 ? __audit_syscall_exit+0x151/0x2a0 ? SyS_ioctl+0x79/0x90 ? __audit_syscall_exit+0x1f6/0x2a0 ? system_call_fastpath+0x16/0x1b ---[ end trace 86461bbc6e1418cd ]--- Does that make sense to anybody? Linus ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PULL] drm-intel-fixes
On Fri, Aug 8, 2014 at 1:52 PM, Linus Torvalds torva...@linux-foundation.org wrote: Got this while bisecting. I'm not sure it's related It's not. The actual bug was panel self refresh. It's still broken, and doesn't work. So enabling it by default was a big mistake (commit b6d547791fd3: drm/i915: Enable PSR by default.) I've reverted that commit, you guys can try again with PSR for the next kernel release if somebody figures out how to get the damn thing *out* of panel self-refresh (because that's what I'm assuming is going on: the graphics may still work fine, but nothing updates on the panel any more). Linus ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 45/53] drm/i915/bdw: Do not call intel_runtime_pm_get() in an interrupt
On Friday, August 08, 2014 11:37:01 AM Daniel Vetter wrote:
On Fri, Aug 08, 2014 at 10:20:40AM +0100, Chris Wilson wrote:
On Tue, Jul 29, 2014 at 12:26:36PM +0200, Daniel Vetter wrote:
On Tue, Jul 29, 2014 at 08:37:48AM +0100, Chris Wilson wrote:
On Mon, Jul 28, 2014 at 10:54:06AM +0200, Daniel Vetter wrote:
On Sat, Jul 26, 2014 at 11:27:38AM +0100, Chris Wilson wrote:
On Wed, Jun 18, 2014 at 10:54:13PM +0200, Daniel Vetter wrote:
On Fri, Jun 13, 2014 at 04:38:03PM +0100, oscar.ma...@intel.com
wrote:
From: Oscar Mateo oscar.ma...@intel.com
Or with a spinlock grabbed, because it might sleep, which is not
a nice thing to do. Instead, do the runtime_pm get/put together
with the create/destroy request, and handle the forcewake
get/put
directly.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
Looks like a fixup that should be squashed into relevant earlier
patches.
The whole gen6_gt_force_wake_get() calling intel_runtime_pm_get() is
broken due to this - we must be able to read registers in atomic
context!
Please revert c8c8fb33b37766acf6474784b0d5245dab9a1690
force_wake_get can't call runtime_pm_get becuase pm_get can sleep. So
if
you want to read registers from atomic context you have to have a
runtime
pm reference from someone else.
Nope. That cannot work.
Well it works currently. So where do you see the problem?
Sampling registers from an timer - in particular, we really do not want
to disable runtime pm whilst trying to monitor the impact of runtime pm.
In that case you can grab a runtime pm reference iff the device is powered
on already. Which won't call anything scary, just amounts to an
atomic_add_unless or so, and then drop it again.
Unfortunately there doesn't seem to be such a thing around already, so
need to add it first. Greg, how much would you freak out if we add
something like
/**
* pm_runtime_get_unless_suspended - grab a rpm ref if the device is on
*
* Returns true if an rpm ref has been acquire, false otherwise. Can be
* called from atomic context to e.g. sample perfomance counters (where we
* obviously don't want to disturb system state if everything is off atm).
*/
static inline bool pm_runtime_get_unless_suspended(struct device *dev)
{
return atomic_add_unless(dev-power.usage_count, 1, 0);
}
I don't think it'll work universally.
That'd need to be synchronized with other stuff done under the spinlock
and in fact, what you're interested in is runtime_status (and that being
RPM_ACTIVE) and not just the usage count.
Rafael
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 45/53] drm/i915/bdw: Do not call intel_runtime_pm_get() in an interrupt
On Friday, August 08, 2014 06:41:10 AM Greg KH wrote:
On Fri, Aug 08, 2014 at 11:37:01AM +0200, Daniel Vetter wrote:
On Fri, Aug 08, 2014 at 10:20:40AM +0100, Chris Wilson wrote:
On Tue, Jul 29, 2014 at 12:26:36PM +0200, Daniel Vetter wrote:
On Tue, Jul 29, 2014 at 08:37:48AM +0100, Chris Wilson wrote:
On Mon, Jul 28, 2014 at 10:54:06AM +0200, Daniel Vetter wrote:
On Sat, Jul 26, 2014 at 11:27:38AM +0100, Chris Wilson wrote:
On Wed, Jun 18, 2014 at 10:54:13PM +0200, Daniel Vetter wrote:
On Fri, Jun 13, 2014 at 04:38:03PM +0100, oscar.ma...@intel.com
wrote:
From: Oscar Mateo oscar.ma...@intel.com
Or with a spinlock grabbed, because it might sleep, which is
not
a nice thing to do. Instead, do the runtime_pm get/put
together
with the create/destroy request, and handle the forcewake
get/put
directly.
Signed-off-by: Oscar Mateo oscar.ma...@intel.com
Looks like a fixup that should be squashed into relevant
earlier patches.
The whole gen6_gt_force_wake_get() calling intel_runtime_pm_get()
is
broken due to this - we must be able to read registers in atomic
context!
Please revert c8c8fb33b37766acf6474784b0d5245dab9a1690
force_wake_get can't call runtime_pm_get becuase pm_get can sleep.
So if
you want to read registers from atomic context you have to have a
runtime
pm reference from someone else.
Nope. That cannot work.
Well it works currently. So where do you see the problem?
Sampling registers from an timer - in particular, we really do not want
to disable runtime pm whilst trying to monitor the impact of runtime pm.
In that case you can grab a runtime pm reference iff the device is powered
on already. Which won't call anything scary, just amounts to an
atomic_add_unless or so, and then drop it again.
Unfortunately there doesn't seem to be such a thing around already, so
need to add it first. Greg, how much would you freak out if we add
something like
/**
* pm_runtime_get_unless_suspended - grab a rpm ref if the device is on
*
* Returns true if an rpm ref has been acquire, false otherwise. Can be
* called from atomic context to e.g. sample perfomance counters (where we
* obviously don't want to disturb system state if everything is off atm).
*/
static inline bool pm_runtime_get_unless_suspended(struct device *dev)
{
return atomic_add_unless(dev-power.usage_count, 1, 0);
}
I'd freak out a lot :)
Rafael, isn't there some other better way to resolve this issue without
resorting to something as horrid as the above?
I'm not sure how to solve this at all, because the above isn't going to work
in general in my opinion.
Can anyone please try to explain the problem to me without referring to the
i915 internals?
Rafael
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Update gen8_ppgtt_info to keep working in full ppgtt
On Fri, Aug 08, 2014 at 03:47:54PM +0200, Daniel Vetter wrote: On Fri, Aug 08, 2014 at 02:10:32PM +0100, Michel Thierry wrote: The driver will no longer initialize the aliasing ppgtt if we have full ppgtt enabled. gen8_ppgtt_info uses the aliasing ppgtt or the ppgtt from the default context. This patch makes it clear. Cc: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Michel Thierry michel.thie...@intel.com I actually looked at this one here and decided that ppgtt_info for gen8 is sufficiently broken so that we don't care. Imo we should refactor it a bit so that it does the same as the gen6 version, i.e. dump the aliasing ppgtt or the contexts. Trying to dump the default ctx ppgtt when full ppgtt is enabled is fairly usesless since nothing at all will ever use that one. Best approach is probably to remove the gen8 version and push the GenX checks down into the actual ppgtt dump functions or something like that. -Daniel I did start down this path here: http://patchwork.freedesktop.org/patch/25728/ (originally here: http://patchwork.freedesktop.org/patch/22348/) It also helped with the later addition for dynamic page tables. I would ask that someone look at fixing that patch to their needs should the dynamic page table patches ever get merged. --- drivers/gpu/drm/i915/i915_debugfs.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 8dc82c3..a62ac66 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1995,8 +1995,14 @@ static void gen8_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 i915_hw_ppgtt *ppgtt = dev_priv-mm.aliasing_ppgtt; + struct intel_context *ctx = dev_priv-ring[RCS].default_context; int unused, i; + if (USES_FULL_PPGTT(dev)) + ppgtt = ctx-ppgtt; + else + ppgtt = dev_priv-mm.aliasing_ppgtt; + if (!ppgtt) return; -- 2.0.3 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Update PSR on resume.
From: Rodrigo Vivi rodrigo.v...@gmail.com
Some registers set during setup might not be persistent after suspend/resume.
This was causing bugs for some people that was unable to get PSR entry state
after resume cycle.
v2: Adding some comments and better commit message explaining why this is
needed.
v3: Getting back old setup_done variable and move from resume to crtc_enable
as Daniel requested.
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 5 +
drivers/gpu/drm/i915/intel_dp.c | 11 ---
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 125a83c..9ef8dfc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -674,6 +674,7 @@ struct i915_psr {
struct mutex lock;
bool sink_support;
bool source_ok;
+ bool setup_done;
struct intel_dp *enabled;
bool active;
struct delayed_work work;
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 89e0ac5..5f8e4f6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3972,6 +3972,11 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE);
POSTING_READ(DSPCNTR(plane));
+ /* Forcing a full psr init sequence when enabling crtc to make sure all
+ * registers are properly set. Some might not be persistent after
+ * suspend/resume cycle. */
+ dev_priv-psr.setup_done = false;
+
dev_priv-display.update_primary_plane(crtc, crtc-primary-fb,
crtc-x, crtc-y);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 34e3c47..5fde763 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1709,6 +1709,9 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp)
struct drm_i915_private *dev_priv = dev-dev_private;
struct edp_vsc_psr psr_vsc;
+ if (dev_priv-psr.setup_done)
+ return;
+
/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
memset(psr_vsc, 0, sizeof(psr_vsc));
psr_vsc.sdp_header.HB0 = 0;
@@ -1720,6 +1723,8 @@ static void intel_edp_psr_setup(struct intel_dp *intel_dp)
/* Avoid continuous PSR exit by masking memup and hpd */
I915_WRITE(EDP_PSR_DEBUG_CTL(dev), EDP_PSR_DEBUG_MASK_MEMUP |
EDP_PSR_DEBUG_MASK_HPD | EDP_PSR_DEBUG_MASK_LPSP);
+
+ dev_priv-psr.setup_done = true;
}
static void intel_edp_psr_enable_sink(struct intel_dp *intel_dp)
@@ -1839,6 +1844,9 @@ static void intel_edp_psr_do_enable(struct intel_dp
*intel_dp)
WARN_ON(dev_priv-psr.active);
lockdep_assert_held(dev_priv-psr.lock);
+ /* Setup PSR once */
+ intel_edp_psr_setup(intel_dp);
+
/* Enable PSR on the panel */
intel_edp_psr_enable_sink(intel_dp);
@@ -1872,9 +1880,6 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp)
dev_priv-psr.busy_frontbuffer_bits = 0;
- /* Setup PSR once */
- intel_edp_psr_setup(intel_dp);
-
if (intel_edp_psr_match_conditions(intel_dp))
dev_priv-psr.enabled = intel_dp;
mutex_unlock(dev_priv-psr.lock);
--
1.9.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 45/53] drm/i915/bdw: Do not call intel_runtime_pm_get() in an interrupt
On Sat, 9 Aug 2014, Rafael J. Wysocki wrote:
Well it works currently. So where do you see the problem?
Sampling registers from an timer - in particular, we really do not want
to disable runtime pm whilst trying to monitor the impact of runtime pm.
In that case you can grab a runtime pm reference iff the device is powered
on already. Which won't call anything scary, just amounts to an
atomic_add_unless or so, and then drop it again.
Unfortunately there doesn't seem to be such a thing around already, so
need to add it first. Greg, how much would you freak out if we add
something like
/**
* pm_runtime_get_unless_suspended - grab a rpm ref if the device is on
*
* Returns true if an rpm ref has been acquire, false otherwise. Can be
* called from atomic context to e.g. sample perfomance counters (where we
* obviously don't want to disturb system state if everything is off atm).
*/
static inline bool pm_runtime_get_unless_suspended(struct device *dev)
{
return atomic_add_unless(dev-power.usage_count, 1, 0);
}
I don't think it'll work universally.
That'd need to be synchronized with other stuff done under the spinlock
and in fact, what you're interested in is runtime_status (and that being
RPM_ACTIVE) and not just the usage count.
That's right. You'd need to acquire the spinlock, test runtime_status,
do the register sampling if the status is RPM_ACTIVE, and then drop the
spinlock.
I suppose wrapper routines for acquiring and releasing the spinlock
could be added to the runtime-PM API. Something like this:
#define pm_runtime_lock(dev, flags) \
spin_lock_irqsave((dev)-power.lock, flags)
#define pm_runtime_unlock(dev, flags) \
spin_unlock_irqrestore((dev)-power.lock, flags)
It looks a little silly but it would work.
Alan Stern
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
