Re: [Intel-gfx] [PATCH] drm/i915: don't setup hdmi for port D edp in ddi_init
On Sat, May 04, 2013 at 11:45:33AM +0200, Daniel Vetter wrote: On Wed, Apr 10, 2013 at 11:28:35PM +0200, Daniel Vetter wrote: dp_init_connector adjusts the encoder type if it is a eDP panel. Use that to decide whether we should set up a hdmi connector or not. To do so reorder the hdmi connector setup sequence in ddi_init a bit. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Does what it says on the tin and works as advertised on my deskop hsw machine with edp on port D. Can someone please review? Looks all right to me. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] drm/i915 stable patches
Dear stable team, Please backport commit 4615d4c9e27eda42c3e965f208a4b4065841498c Author: Chris Wilson ch...@chris-wilson.co.uk Date: Mon Apr 8 14:28:40 2013 +0100 drm/i915: Use MLC (l3$) for context objects to all supported stable kernels. It's not just a performance optimization, but seems to prevent gpu hangs, too. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64073 Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] drm/i915 stable patches
On Mon, May 6, 2013 at 10:33 AM, Daniel Vetter daniel.vet...@ffwll.ch wrote: Dear stable team, Please backport commit 4615d4c9e27eda42c3e965f208a4b4065841498c Author: Chris Wilson ch...@chris-wilson.co.uk Date: Mon Apr 8 14:28:40 2013 +0100 drm/i915: Use MLC (l3$) for context objects to all supported stable kernels. It's not just a performance optimization, but seems to prevent gpu hangs, too. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64073 And another one for 3.8 and older: commit e12a2d53ae45a69aea499b64f75e7222cca0f12f Author: Chris Wilson ch...@chris-wilson.co.uk Date: 2012-11-15 11:32:18 + drm/i915: Fix detection of base of stolen memory Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=53541 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] drm/i915: don't setup hdmi for port D edp in ddi_init
On Mon, May 06, 2013 at 11:31:57AM +0300, Ville Syrjälä wrote: On Sat, May 04, 2013 at 11:45:33AM +0200, Daniel Vetter wrote: On Wed, Apr 10, 2013 at 11:28:35PM +0200, Daniel Vetter wrote: dp_init_connector adjusts the encoder type if it is a eDP panel. Use that to decide whether we should set up a hdmi connector or not. To do so reorder the hdmi connector setup sequence in ddi_init a bit. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Does what it says on the tin and works as advertised on my deskop hsw machine with edp on port D. Can someone please review? Looks all right to me. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Queued for -next, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: add context into request struct
On Fri, May 03, 2013 at 05:07:42PM -0700, Ben Widawsky wrote: On Thu, May 02, 2013 at 04:48:08PM +0300, Mika Kuoppala wrote: Storing context reference into request struct allows us to inspect context and its associated objects when requests are retired. Both ppgtt and arb robustness work will need this. Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com Both 12 are: Reviewed-by: Ben Widawsky b...@bwidawsk.net You should add your sob to 1, since you modified it (slightly), and maybe run them both my Chris to make sure he approves as well. Thanks a lot of reworking the series to this order :D Both patches merged to dinq, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915: fix up adjusted_mode tracking for interlaced modes
On Fri, May 03, 2013 at 11:31:46AM -0300, Paulo Zanoni wrote: 2013/5/3 Daniel Vetter daniel.vet...@ffwll.ch: On Fri, May 3, 2013 at 11:49 AM, Daniel Vetter daniel.vet...@ffwll.ch wrote: With the hw state readoutcheck code it's important that the values we keep around are the canonical ones. Unfortunately when adding the pipe timings readout support I've missed that the write side adjusts the timings in the pipe config. Fix this up. Reported-by: Paulo Zanoni przan...@gmail.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch I better be a good example ;-) So: This fixes a regression introduced in commit 1bd1bd806037af04dd1d7bdd39b2b04090c10d2c Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Mon Apr 29 21:56:12 2013 +0200 drm/i915: hw state readout support for pipe timings Cc: Mika Kuoppala mika.kuopp...@intel.com To make it super complete, you could also explicitly mention that this fixes a WARN when we use interlaced modes :) So now I booted it with the eDP + DP-interlaced I was using yesterday and I don't see the error message anymore. Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com Tested-by: Paulo Zanoni paulo.r.zan...@intel.com Queued for -next, thanks for the review and testing. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/i915: PCH_ prefix for transcoder timings
On Fri, May 03, 2013 at 11:16:48AM -0300, Paulo Zanoni wrote: 2013/5/3 Daniel Vetter daniel.vet...@ffwll.ch: While at it, also extract a common helper to copy the timings from the cpu transcoder to the pch transcoder. That way it's really explicit how the lpt transcoder is hardcoded. v2: - Re-align #defines properly (Paulo). - Use cpu_transcoder when copying pipe timings (Paulo). - s/intel_pch_transcoder_enable/intel_pch_transcoder_set_timings/ since we already have a pch transcoder enable function, and this is clearer, too. - Fixup 80 char line overflow in intel_display.c. I've opted to ignore this in i915_reg.h and i915_ums.c since meh. Cc: Paulo Zanoni przan...@gmail.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com All patches from this series are now merged, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: add context into request struct
Ben Widawsky b...@bwidawsk.net writes: On Thu, May 02, 2013 at 04:48:08PM +0300, Mika Kuoppala wrote: Storing context reference into request struct allows us to inspect context and its associated objects when requests are retired. Both ppgtt and arb robustness work will need this. Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com Both 12 are: Reviewed-by: Ben Widawsky b...@bwidawsk.net You should add your sob to 1, since you modified it (slightly), and maybe run them both my Chris to make sure he approves as well. I took 1/2 from: http://cgit.freedesktop.org/~bwidawsk/drm-intel/commit/?h=ppgtt-ctxid=5e266650d53d42ebbc8c22f2846c8ed87d747b21 and i don't remember intentionally modifying it. Is there some merge fallout I missed? I couldn't spot any diff to original. -Mika ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/12] drm/i915: Assert mutex_is_locked on context lookup
On Thu, May 02, 2013 at 01:27:32PM -0700, Jesse Barnes wrote: On Tue, 23 Apr 2013 23:15:29 -0700 Ben Widawsky b...@bwidawsk.net wrote: Because our context refcounting doesn't grab a ref at lookup time, it is unsafe to do so without the lock. NOTE: We don't have an easy way to put the assertion in the lookup function which is where this really belongs. Context switching is good enough because it actually asserts even more correctness by protecting the default_context. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index a1e8ecb..411ace0 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -444,6 +444,8 @@ int i915_switch_context(struct intel_ring_buffer *ring, if (dev_priv-hw_contexts_disabled) return 0; + BUG_ON(!mutex_is_locked(dev_priv-dev-struct_mutex)); + if (ring != dev_priv-ring[RCS]) return 0; Simple enough. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org We usually do WARN_ONs for this stuff though, in case a user actually does hit it, it may not be fatal so why crash the machine? But that's a minor distinction since we shouldn't hit this except in development anyway. Well, since this is a patch for upstream the focus should very much be on supporting bug reporters and not developers. And for bug reporters a BUG is much more annoying than a WARN and greatly reduces the chances that we'll get a bug report. There are imo only very few cases where a BUG instead of a WARN is justified: - The kernel is _guaranteed_ to oops in the next few lines anyway, so a BUG_ON will help in readability of the backtrace. Note that checking 3 different things for non-NULL in the same BUG actually reduces OOPS readability (with an oops you can at least reconstruct the faulting address and so probably the pointer). Also, this means the BUG should have a neat description of what exactly blew up. The a few lines part is just a guideline with some big exceptions. A prime example is refcount over/underflows since those will blow up, but only sometimes later (and usually no one will have a clue why). - BUGs are justified if there's a potential security hole awaiting, e.g. when something in the userspace input validation has gone wrong. - I'm wary of special error handling for WARNs, but if an early return (with an error code if possible) transforms a BUG into a WARN I'm in. But trying to fix up e.g. modeset state is usually futile, since we'll end up with a black/fuzzy/corrupted screen most likely anyway. But the most important rule is: In case of doubt, just WARN, don't BUG. [I know, I violate it sometimes, too.] Patch applied with the s/BUG/WARN bikeshed. -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 01/12] drm/i915: Assert mutex_is_locked on context lookup
On Mon, May 06, 2013 at 11:40:06AM +0200, Daniel Vetter wrote: On Thu, May 02, 2013 at 01:27:32PM -0700, Jesse Barnes wrote: On Tue, 23 Apr 2013 23:15:29 -0700 Ben Widawsky b...@bwidawsk.net wrote: Because our context refcounting doesn't grab a ref at lookup time, it is unsafe to do so without the lock. NOTE: We don't have an easy way to put the assertion in the lookup function which is where this really belongs. Context switching is good enough because it actually asserts even more correctness by protecting the default_context. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index a1e8ecb..411ace0 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -444,6 +444,8 @@ int i915_switch_context(struct intel_ring_buffer *ring, if (dev_priv-hw_contexts_disabled) return 0; + BUG_ON(!mutex_is_locked(dev_priv-dev-struct_mutex)); + if (ring != dev_priv-ring[RCS]) return 0; Simple enough. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org We usually do WARN_ONs for this stuff though, in case a user actually does hit it, it may not be fatal so why crash the machine? But that's a minor distinction since we shouldn't hit this except in development anyway. Well, since this is a patch for upstream the focus should very much be on supporting bug reporters and not developers. And for bug reporters a BUG is much more annoying than a WARN and greatly reduces the chances that we'll get a bug report. Some more details why a WARN massively beats a BUG for us: BUG kills the current process and ensures all locks are stuck. Usually that means X is dead and you can't vt-switch away to the console to take a quick look at dmesg. Now even when all rendering is down the toilet due to the follow-up damage after the WARN and the gpu a zombie, there's a non-zero chance that vt-switch (or sw rendering in X) will work long enough to grab log files and debugfs data. Hence the first rule to only use a BUG on if we have a guaranteed OOPS otherwise (which again will kill the process and make all locks stuck). -Daniel There are imo only very few cases where a BUG instead of a WARN is justified: - The kernel is _guaranteed_ to oops in the next few lines anyway, so a BUG_ON will help in readability of the backtrace. Note that checking 3 different things for non-NULL in the same BUG actually reduces OOPS readability (with an oops you can at least reconstruct the faulting address and so probably the pointer). Also, this means the BUG should have a neat description of what exactly blew up. The a few lines part is just a guideline with some big exceptions. A prime example is refcount over/underflows since those will blow up, but only sometimes later (and usually no one will have a clue why). - BUGs are justified if there's a potential security hole awaiting, e.g. when something in the userspace input validation has gone wrong. - I'm wary of special error handling for WARNs, but if an early return (with an error code if possible) transforms a BUG into a WARN I'm in. But trying to fix up e.g. modeset state is usually futile, since we'll end up with a black/fuzzy/corrupted screen most likely anyway. But the most important rule is: In case of doubt, just WARN, don't BUG. [I know, I violate it sometimes, too.] Patch applied with the s/BUG/WARN bikeshed. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- 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 02/12] drm/i915: BUG_ON bad PPGTT offset
On Thu, May 02, 2013 at 01:28:23PM -0700, Jesse Barnes wrote:
On Tue, 23 Apr 2013 23:15:30 -0700
Ben Widawsky b...@bwidawsk.net wrote:
Because PPGTT PDEs within the GTT are calculated in cachelines
(HW guys consistency ftw) we do a divide which will wreak havoc if this
is wrong, and I know that from experience).
If/when we move to multiple PPGTTs this will have to become a WARN, and
return an error. For now however it should always be considered fatal,
and only a developer could hit it.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 50df194..0503f09 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -85,6 +85,8 @@ static int gen6_ppgtt_enable(struct drm_device *dev)
uint32_t pd_entry;
int i;
+ BUG_ON(ppgtt-pd_offset 0x3f);
+
pd_addr = (gen6_gtt_pte_t __iomem*)dev_priv-gtt.gsm +
ppgtt-pd_offset / sizeof(gen6_gtt_pte_t);
for (i = 0; i ppgtt-num_pd_entries; i++) {
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Queued for -next with the same s/BUG/WARN bikeshed, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/12] drm/i915: Extract PDE writes
On Thu, May 02, 2013 at 02:27:41PM -0700, Jesse Barnes wrote:
On Tue, 23 Apr 2013 23:15:32 -0700
Ben Widawsky b...@bwidawsk.net wrote:
It also makes some sense IMO to have these two functions separate
irrespective of the number of callers.
Only the single caller for now, but that will change as we add more
PPGTTs.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 11a50cf..975adaa 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -76,18 +76,13 @@ static inline gen6_gtt_pte_t gen6_pte_encode(struct
drm_device *dev,
return pte;
}
-static int gen6_ppgtt_enable(struct drm_device *dev)
+static void gen6_write_pdes(struct i915_hw_ppgtt *ppgtt)
{
- drm_i915_private_t *dev_priv = dev-dev_private;
- uint32_t pd_offset;
- struct intel_ring_buffer *ring;
- struct i915_hw_ppgtt *ppgtt = dev_priv-mm.aliasing_ppgtt;
+ struct drm_i915_private *dev_priv = ppgtt-dev-dev_private;
gen6_gtt_pte_t __iomem *pd_addr;
uint32_t pd_entry;
int i;
- BUG_ON(ppgtt-pd_offset 0x3f);
-
pd_addr = (gen6_gtt_pte_t __iomem*)dev_priv-gtt.gsm +
ppgtt-pd_offset / sizeof(gen6_gtt_pte_t);
for (i = 0; i ppgtt-num_pd_entries; i++) {
@@ -100,6 +95,19 @@ static int gen6_ppgtt_enable(struct drm_device *dev)
writel(pd_entry, pd_addr + i);
}
readl(pd_addr);
+}
+
+static int gen6_ppgtt_enable(struct drm_device *dev)
+{
+ drm_i915_private_t *dev_priv = dev-dev_private;
+ uint32_t pd_offset;
+ struct intel_ring_buffer *ring;
+ struct i915_hw_ppgtt *ppgtt = dev_priv-mm.aliasing_ppgtt;
+ int i;
+
+ BUG_ON(ppgtt-pd_offset 0x3f);
+
+ gen6_write_pdes(ppgtt);
pd_offset = ppgtt-pd_offset;
pd_offset /= 64; /* in cachelines, */
Yep, looks nicer this way.
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/9] drm/i915: fix linetime_watermarks code
2013/5/5 Chris Wilson ch...@chris-wilson.co.uk:
On Fri, May 03, 2013 at 05:23:38PM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
The spec says the linetime watermarks must be programmed before
enabling any display low power watermarks, but we're currently
updating the linetime watermarks after we call intel_update_watermarks
(and only at crtc_mode_set, not at crtc_{enable,disable}). So IMHO the
best way guarantee the linetime watermarks will be updated before the
low power watermarks is inside the update_wm function, because it's
the function that enables low power watermarks. And since Haswell is
the only platform that has linetime watermarks, let's completely kill
the intel_update_linetime_watermarks abstraction and just use the
intel_update_watermarks abstraction by creating haswell_update_wm.
+static void haswell_update_wm(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct drm_crtc *crtc;
+ enum pipe pipe;
+
+ for_each_pipe(pipe) {
+ crtc = dev_priv-pipe_to_crtc_mapping[pipe];
+ haswell_update_linetime_wm(dev, crtc);
+ }
The LP watermarks are still enabled at this point.
Did you mean they're not disabled yet? Well, at least now we're
programming the linetime registers _before_ we update the LP
registers, and every time. If we want to be 100% spec-compliant
without any intermediate steps then I guess we can discard this
linetime series and merge it all inside the single patch that will
fully implement haswell_update_wm. But it will be one single big
patch. Just tell me which one you prefer.
+
+ sandybridge_update_wm(dev);
+}
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices
On Tue, 30 Apr 2013, Daniel Vetter daniel.vet...@ffwll.ch wrote:
We need to track this correctly. While at it shovel the boolean
to track whether the sdvo is in tv mode or not into pipe_config.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36997
Bug scrubbing, should this be:
https://bugs.freedesktop.org/show_bug.cgi?id=63609 ?
Jani.
Tested-by: Pierre Assal pierre.as...@verint.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36997
Tested-by: cancan,feng cancan.f...@intel.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
drivers/gpu/drm/i915/intel_display.c | 8 +++-
drivers/gpu/drm/i915/intel_drv.h | 5 -
drivers/gpu/drm/i915/intel_sdvo.c| 8 ++--
3 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 44bcfae..ef0d27b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4546,7 +4546,7 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
if (INTEL_INFO(dev)-gen = 4)
dpll |= (6 PLL_LOAD_PULSE_PHASE_SHIFT);
- if (is_sdvo intel_pipe_has_type(crtc-base, INTEL_OUTPUT_TVOUT))
+ if (crtc-config.sdvo_tv_clock)
dpll |= PLL_REF_INPUT_TVCLKINBC;
else if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_LVDS)
intel_panel_use_ssc(dev_priv) num_connectors 2)
@@ -5590,7 +5590,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc
*intel_crtc,
struct intel_encoder *intel_encoder;
uint32_t dpll;
int factor, num_connectors = 0;
- bool is_lvds = false, is_sdvo = false, is_tv = false;
+ bool is_lvds = false, is_sdvo = false;
for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
switch (intel_encoder-type) {
@@ -5600,8 +5600,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc
*intel_crtc,
case INTEL_OUTPUT_SDVO:
case INTEL_OUTPUT_HDMI:
is_sdvo = true;
- if (intel_encoder-needs_tv_clock)
- is_tv = true;
break;
}
@@ -5615,7 +5613,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc
*intel_crtc,
dev_priv-lvds_ssc_freq == 100) ||
(HAS_PCH_IBX(dev) intel_is_dual_link_lvds(dev)))
factor = 25;
- } else if (is_sdvo is_tv)
+ } else if (intel_crtc-config.sdvo_tv_clock)
factor = 20;
if (ironlake_needs_fb_cb_tune(intel_crtc-config.dpll, factor))
diff --git a/drivers/gpu/drm/i915/intel_drv.h
b/drivers/gpu/drm/i915/intel_drv.h
index 766afcf..be196ff 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -120,7 +120,6 @@ struct intel_encoder {
struct intel_crtc *new_crtc;
int type;
- bool needs_tv_clock;
/*
* Intel hw has only one MUX where encoders could be clone, hence a
* simple flag is enough to compute the possible_clones mask.
@@ -223,6 +222,10 @@ struct intel_crtc_config {
/* Controls for the clock computation, to override various stages. */
bool clock_set;
+ /* SDVO TV has a bunch of special case. To make multifunction encoders
+ * work correctly, we need to track this at runtime.*/
+ bool sdvo_tv_clock;
+
/*
* crtc bandwidth limit, don't increase pipe bpp or clock if not really
* required. This is set in the 2nd loop of calling encoder's
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
b/drivers/gpu/drm/i915/intel_sdvo.c
index f6bf9fc..3a1d710 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1092,6 +1092,7 @@ static bool intel_sdvo_compute_config(struct
intel_encoder *encoder,
(void) intel_sdvo_get_preferred_input_mode(intel_sdvo,
mode,
adjusted_mode);
+ pipe_config-sdvo_tv_clock = true;
} else if (intel_sdvo-is_lvds) {
if (!intel_sdvo_set_output_timings_from_mode(intel_sdvo,
intel_sdvo-sdvo_lvds_fixed_mode))
@@ -1655,12 +1656,9 @@ intel_sdvo_detect(struct drm_connector *connector,
bool force)
if (ret == connector_status_connected) {
intel_sdvo-is_tv = false;
intel_sdvo-is_lvds = false;
- intel_sdvo-base.needs_tv_clock = false;
- if (response SDVO_TV_MASK) {
+ if (response SDVO_TV_MASK)
intel_sdvo-is_tv = true;
- intel_sdvo-base.needs_tv_clock = true;
- }
if (response SDVO_LVDS_MASK)
intel_sdvo-is_lvds = intel_sdvo-sdvo_lvds_fixed_mode
!=
Re: [Intel-gfx] [PATCH 2/9] drm/i915: fix linetime_watermarks code
On Mon, May 06, 2013 at 10:13:33AM -0300, Paulo Zanoni wrote:
2013/5/5 Chris Wilson ch...@chris-wilson.co.uk:
On Fri, May 03, 2013 at 05:23:38PM -0300, Paulo Zanoni wrote:
From: Paulo Zanoni paulo.r.zan...@intel.com
The spec says the linetime watermarks must be programmed before
enabling any display low power watermarks, but we're currently
updating the linetime watermarks after we call intel_update_watermarks
(and only at crtc_mode_set, not at crtc_{enable,disable}). So IMHO the
best way guarantee the linetime watermarks will be updated before the
low power watermarks is inside the update_wm function, because it's
the function that enables low power watermarks. And since Haswell is
the only platform that has linetime watermarks, let's completely kill
the intel_update_linetime_watermarks abstraction and just use the
intel_update_watermarks abstraction by creating haswell_update_wm.
+static void haswell_update_wm(struct drm_device *dev)
+{
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct drm_crtc *crtc;
+ enum pipe pipe;
+
+ for_each_pipe(pipe) {
+ crtc = dev_priv-pipe_to_crtc_mapping[pipe];
+ haswell_update_linetime_wm(dev, crtc);
+ }
The LP watermarks are still enabled at this point.
Did you mean they're not disabled yet? Well, at least now we're
programming the linetime registers _before_ we update the LP
registers, and every time. If we want to be 100% spec-compliant
without any intermediate steps then I guess we can discard this
linetime series and merge it all inside the single patch that will
fully implement haswell_update_wm. But it will be one single big
patch. Just tell me which one you prefer.
Or you can just disable the 3 LP wm first before updating the linetime
wm.
-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][PATCH 0/1] drm/i915: Big watermark changes
This is a rather messy diff resulting from my attempt at making our watermark handling more robust, and especially more suitable for the atomic age. I'll have to work on splitting the diff into some more sensible pieces, but I would appreciate some early comments on the general approach. But for that, it's possibly better to look at the resulting code [1] rather than the crappy diff. [1] git://gitorious.org/vsyrjala/linux.git watermark2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/9] drm/i915: make intel_ddi_get_cdclk_freq return values in KHz
2013/5/5 Chris Wilson ch...@chris-wilson.co.uk: On Fri, May 03, 2013 at 05:23:42PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com With this, that 338 can finally become the correct 337500. Due to the change we need to adjust the intel_dp_aux_ch function to set the correct value, so adjust the division and also use DIV_ROUND_CLOSEST instead of the old round down behavior because the spec says the value should be programmed to get as close as possible to the ideal rate of 2MHz. Can you please demonstrate an instance where this code produces a different value? And only then correct the constants. I use the 337500 value on the next patch, when setting the ips_linetime value. The correct frequency is 337500, not 338000. ips_linetime = DIV_ROUND_CLOSEST(mode-htotal * 1000 * 8, intel_ddi_get_cdclk_freq); For a mode with htotal of 2640 [0] we'll have: (i) (2640 * 1000 * 8) / 338000 = 62.48, resulting in 62 and (ii) (2640 * 1000 * 8) / 337500 = 62.57 resulting in 63. For the case inside intel_dp.c: Previously we were using 338. So with the old formula we were writing 338/2 = 169 to the register. And 337500 / 169 = 1997.04 (we use 337500 here because it's the real clock value). With the new value of 337500/2000 we'll have 168.75, which is 168 on the round-down case and 169 on the round-closest case. If we write 168 to the register, 337500 / 168 = 2008.92, and 2008.92 is more distant from 2000 than 1997.04. So with this patch we're changing the formula but still writing the same correct value to the DP AUX register. [0]: That's 1920x1080@50Hz on my DP monitor. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/7] drm/i915: make SDVO TV-out work for multifunction devices
On Mon, May 6, 2013 at 3:36 PM, Jani Nikula jani.nik...@linux.intel.com wrote: On Tue, 30 Apr 2013, Daniel Vetter daniel.vet...@ffwll.ch wrote: We need to track this correctly. While at it shovel the boolean to track whether the sdvo is in tv mode or not into pipe_config. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36997 Bug scrubbing, should this be: https://bugs.freedesktop.org/show_bug.cgi?id=63609 ? Yeah, I've copypasted the wrong bugzilla link, that's the 2nd one. Thanks for spotting. -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] [TRIVIAL] v2: Fix declaration of intel_gmbus_{is_forced_bit/is_port_falid} in i915 driver.
On Mon, May 06, 2013 at 02:52:08PM +0200, dl...@gmx.de wrote: From: Jan-Simon Möller dl...@gmx.de 20130509: v2: (re-)add inline upon request. Description: intel_gmbus_is_forced_bit is no extern as its body is right below. Likewise for intel_gmbus_is_port_valid. This fixes a compilation issue with clang. An initial version of this patch was developed by PaX Team pageexec at freemail.hu. This is respin of this patch. Signed-off-by: Jan-Simon Möller dl...@gmx.de CC: pagee...@freemail.hu CC: daniel.vet...@ffwll.ch CC: airl...@linux.ie CC: intel-gfx@lists.freedesktop.org CC: dri-de...@lists.freedesktop.org CC: linux-ker...@vger.kernel.org Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: go back to switch for VLV mem freq detection v2
On Fri, May 03, 2013 at 10:52:11AM -0700, Ben Widawsky wrote:
On Thu, May 02, 2013 at 10:48:08AM -0700, Jesse Barnes wrote:
Both the docs and the existing code were wrong. So fix both and use a
switch statement like we do elsewhere to make things simple clear.
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_pm.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c
b/drivers/gpu/drm/i915/intel_pm.c
index 0f4b46e..556b989 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2902,7 +2902,18 @@ static void valleyview_enable_rps(struct drm_device
*dev)
GEN7_RC_CTL_TO_MODE);
valleyview_punit_read(dev_priv, PUNIT_REG_GPU_FREQ_STS, val);
- dev_priv-mem_freq = 800 + (266 * (val 6) 3);
+ switch ((val 6) 3) {
+ case 0:
+ case 1:
+ dev_priv-mem_freq = 800;
+ break;
+ case 2:
+ dev_priv-mem_freq = 1066;
+ break;
+ case 3:
+ dev_priv-mem_freq = 1333;
+ break;
+ }
DRM_DEBUG_DRIVER(DDR speed: %d MHz, dev_priv-mem_freq);
DRM_DEBUG_DRIVER(GPLL enabled? %s\n, val 0x10 ? yes : no);
The code does what the author wants it to, but I don't have the doc that
says this:
Reviewed-by: Ben Widawsky b...@bwidawsk.net
First two patches of this series are merged to dinq, thanks.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Clover Trail question...
On Mon, May 6, 2013 at 1:43 PM, Niko! nicola.rana...@gmail.com wrote: Hi Daniel, I'm a damned and fooled linux-only user with a fresh z2760 based tablet/netbook (samsung ativ 500t e.g. FULL PC EXPERIENCE!!!). Before I'll launch it over the window may you tell me if Intel is going to add linux support for it in the next months as for the clover trail +? Are there some underground hidden patches to build a working kernel? I do not need full esoteric drivers support with optimal power management just need to use the system as a classical netbook. I'll keep all the information you'll provide me higly confidential (but will understand if you'll answer me only partially). a) I can't disclose confidential stuff, ever, so b) always cc a mailing list when poking your maintainer, he's a lazy bastard ;-) For your question: This is a poulsbo/gma500 based thing, so no real linux support from Intel's open source group. There's the gma500 driver in the kernel, but that one only supports kms and no accelaration. Or you can frustrate yourself with the blob. I recommend to return that thing and either wait for a baytrail (real linux support there) or buy a cheap non-atom core with intel hd graphics. 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] [TRIVIAL] Fix declaration of intel_gmbus_{is_forced_bit/is_port_falid} in i915 driver.
Ok, so let me resend a version with static inline . Best, JS On Saturday 04 May 2013 10:55:50 PaX Team wrote: On 3 May 2013 at 15:03, Jani Nikula wrote: This fixes a compilation issue with clang. An initial version of this patch was developed by PaX Team pageexec at freemail.hu. This is respin of this patch. Signed-off-by: Jan-Simon Möller dl...@gmx.de CC: pagee...@freemail.hu CC: daniel.vet...@ffwll.ch CC: airl...@linux.ie CC: intel-gfx@lists.freedesktop.org CC: dri-de...@lists.freedesktop.org CC: linux-ker...@vger.kernel.org Picked up for -fixes, thanks for the patch. Please drop it. The patch removes the inline keyword, creating dozens of copies of the functions, and consequently loads of warnings: in my original patch they were both static inline, not sure where the inline got lost... -- Dipl.-Ing. Jan-Simon Möller jansimon.moel...@gmx.de ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix panel fitting on LVDS on ILK+ v2
On Fri, May 03, 2013 at 01:26:37PM -0700, Jesse Barnes wrote: This regression was introduced in: commit b074cec8c652f2d273907a4b35239b4766c894ac Author: Jesse Barnes jbar...@virtuousgeek.org Date: Thu Apr 25 12:55:02 2013 -0700 drm/i915: move PCH pfit controls into pipe_config In refactoring this, it was only applied to eDP, which is incorrect. In fact, if we ever use the panel fitter to deal with overscan on HDMI, we'll need to extend it again, so just drop the conditional altogether. v2: drop check for eDP since we can use the fitter in any config (Daniel) Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: get mode clock when reading the pipe config v3
We need this for comparing modes between configuration changes.
v2: try harder to calulate non-simple pixel clocks (Daniel)
call get_clock after getting the encoder config, needed for pixel multiply
(Jesse)
v3: drop get_clock now that the pixel_multiply has been moved into
get_pipe_config
Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
drivers/gpu/drm/i915/intel_display.c | 96 +++---
drivers/gpu/drm/i915/intel_drv.h |3 ++
2 files changed, 92 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 924932f..c1826cc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -45,6 +45,11 @@ bool intel_pipe_has_type(struct drm_crtc *crtc, int type);
static void intel_increase_pllclock(struct drm_crtc *crtc);
static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on);
+static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
+ struct intel_crtc_config *pipe_config);
+static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
+ struct intel_crtc_config *pipe_config);
+
typedef struct {
int min, max;
} intel_range_t;
@@ -4982,6 +4987,11 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
intel_get_pipe_timings(crtc, pipe_config);
+ /* FIXME: SDVO HDMI pixel repeat */
+ pipe_config-pixel_multiplier = 1;
+
+ i9xx_crtc_clock_get(crtc, pipe_config);
+
return true;
}
@@ -5883,7 +5893,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc
*crtc,
{
struct drm_device *dev = crtc-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
- uint32_t tmp;
+ uint32_t tmp, dpa;
tmp = I915_READ(PIPECONF(crtc-pipe));
if (!(tmp PIPECONF_ENABLE))
@@ -5899,8 +5909,21 @@ static bool ironlake_get_pipe_config(struct intel_crtc
*crtc,
ironlake_get_fdi_m_n_config(crtc, pipe_config);
}
+ dpa = I915_READ(DP_A);
+ if ((dpa DP_PORT_EN) ((dpa DP_PIPE_MASK) 30) == crtc-pipe) {
+ if ((I915_READ(DP_A) DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ)
+ pipe_config-cpu_edp_link_rate = 162000;
+ else
+ pipe_config-cpu_edp_link_rate = 27;
+ }
+
intel_get_pipe_timings(crtc, pipe_config);
+ /* FIXME: SDVO HDMI pixel repeat */
+ pipe_config-pixel_multiplier = 1;
+
+ ironlake_crtc_clock_get(crtc, pipe_config);
+
return true;
}
@@ -6049,6 +6072,11 @@ static bool haswell_get_pipe_config(struct intel_crtc
*crtc,
intel_get_pipe_timings(crtc, pipe_config);
+ /* FIXME: SDVO HDMI pixel repeat */
+ pipe_config-pixel_multiplier = 1;
+
+ ironlake_crtc_clock_get(crtc, pipe_config);
+
return true;
}
@@ -6910,11 +6938,12 @@ void intel_release_load_detect_pipe(struct
drm_connector *connector,
}
/* Returns the clock of the currently programmed mode of the given pipe. */
-static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
+static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
+ struct intel_crtc_config *pipe_config)
{
+ struct drm_device *dev = crtc-base.dev;
struct drm_i915_private *dev_priv = dev-dev_private;
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- int pipe = intel_crtc-pipe;
+ int pipe = pipe_config-cpu_transcoder;
u32 dpll = I915_READ(DPLL(pipe));
u32 fp;
intel_clock_t clock;
@@ -6953,7 +6982,8 @@ static int intel_crtc_clock_get(struct drm_device *dev,
struct drm_crtc *crtc)
default:
DRM_DEBUG_KMS(Unknown DPLL mode %08x in programmed
mode\n, (int)(dpll DPLL_MODE_MASK));
- return 0;
+ pipe_config-adjusted_mode.clock = 0;
+ return;
}
/* XXX: Handle the 100Mhz refclk */
@@ -6992,8 +7022,56 @@ static int intel_crtc_clock_get(struct drm_device *dev,
struct drm_crtc *crtc)
* i830PllIsValid() because it relies on the xf86_config connector
* configuration being accurate, which it isn't necessarily.
*/
+ pipe_config-adjusted_mode.clock = clock.dot;
+}
+
+static void ironlake_crtc_clock_get(struct intel_crtc *crtc,
+ struct intel_crtc_config *pipe_config)
+{
+ struct drm_device *dev = crtc-base.dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ enum transcoder cpu_transcoder = pipe_config-cpu_transcoder;
+ int link_freq, repeat;
+ u64 clock;
+ u32 link_m, link_n;
+
+ /* FIXME: Haswell bits */
+
+ repeat = pipe_config-pixel_multiplier;
+
+ /*
+* The
Re: [Intel-gfx] [PATCH 3/4] drm/i915: set proper DPIO post divider for VGA on VLV v4
On 05/02/2013 10:48 AM, Jesse Barnes wrote: Supposedly we should use the DAC divider for 300MHz pixel clocks, but as that doesn't actually work as well as the high freq divider here in practice, just use the high freq divider all the time. v2: remove unconditional write (Jesse) check for pixel rate properly (Jesse) v3: give up, the DAC divider apparently doesn't work, and low res modes work ok (Jesse) remove debug msg (Jesse) Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org VGA is broken with today's drm-intel-next-queued (3a2128bcac1ae). Applying this patch fixes it. Thanks! Tested-by: Kenneth Graunke kenn...@whitecape.org --- drivers/gpu/drm/i915/intel_display.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6504337..59c2114 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4425,10 +4425,13 @@ static void vlv_update_pll(struct intel_crtc *crtc) mdiv |= ((bestp1 DPIO_P1_SHIFT) | (bestp2 DPIO_P2_SHIFT)); mdiv |= ((bestn DPIO_N_SHIFT)); mdiv |= (1 DPIO_K_SHIFT); - if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_HDMI) || - intel_pipe_has_type(crtc-base, INTEL_OUTPUT_EDP) || - intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DISPLAYPORT)) - mdiv |= (DPIO_POST_DIV_HDMIDP DPIO_POST_DIV_SHIFT); + + /* +* Post divider depends on pixel clock rate, DAC vs digital (and LVDS, +* but we don't support that). +* Note: don't use the DAC post divider as it seems unstable. +*/ + mdiv |= (DPIO_POST_DIV_HDMIDP DPIO_POST_DIV_SHIFT); intel_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv); mdiv |= DPIO_ENABLE_CALIBRATION; ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/12] drm/i915: Assert mutex_is_locked on context lookup
On Mon, May 06, 2013 at 11:44:22AM +0200, Daniel Vetter wrote: On Mon, May 06, 2013 at 11:40:06AM +0200, Daniel Vetter wrote: On Thu, May 02, 2013 at 01:27:32PM -0700, Jesse Barnes wrote: On Tue, 23 Apr 2013 23:15:29 -0700 Ben Widawsky b...@bwidawsk.net wrote: Because our context refcounting doesn't grab a ref at lookup time, it is unsafe to do so without the lock. NOTE: We don't have an easy way to put the assertion in the lookup function which is where this really belongs. Context switching is good enough because it actually asserts even more correctness by protecting the default_context. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index a1e8ecb..411ace0 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -444,6 +444,8 @@ int i915_switch_context(struct intel_ring_buffer *ring, if (dev_priv-hw_contexts_disabled) return 0; + BUG_ON(!mutex_is_locked(dev_priv-dev-struct_mutex)); + if (ring != dev_priv-ring[RCS]) return 0; Simple enough. Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org We usually do WARN_ONs for this stuff though, in case a user actually does hit it, it may not be fatal so why crash the machine? But that's a minor distinction since we shouldn't hit this except in development anyway. Well, since this is a patch for upstream the focus should very much be on supporting bug reporters and not developers. And for bug reporters a BUG is much more annoying than a WARN and greatly reduces the chances that we'll get a bug report. Some more details why a WARN massively beats a BUG for us: BUG kills the current process and ensures all locks are stuck. Usually that means X is dead and you can't vt-switch away to the console to take a quick look at dmesg. Now even when all rendering is down the toilet due to the follow-up damage after the WARN and the gpu a zombie, there's a non-zero chance that vt-switch (or sw rendering in X) will work long enough to grab log files and debugfs data. Hence the first rule to only use a BUG on if we have a guaranteed OOPS otherwise (which again will kill the process and make all locks stuck). -Daniel There are imo only very few cases where a BUG instead of a WARN is justified: - The kernel is _guaranteed_ to oops in the next few lines anyway, so a BUG_ON will help in readability of the backtrace. Note that checking 3 different things for non-NULL in the same BUG actually reduces OOPS readability (with an oops you can at least reconstruct the faulting address and so probably the pointer). Also, this means the BUG should have a neat description of what exactly blew up. The a few lines part is just a guideline with some big exceptions. A prime example is refcount over/underflows since those will blow up, but only sometimes later (and usually no one will have a clue why). - BUGs are justified if there's a potential security hole awaiting, e.g. when something in the userspace input validation has gone wrong. - I'm wary of special error handling for WARNs, but if an early return (with an error code if possible) transforms a BUG into a WARN I'm in. But trying to fix up e.g. modeset state is usually futile, since we'll end up with a black/fuzzy/corrupted screen most likely anyway. But the most important rule is: In case of doubt, just WARN, don't BUG. [I know, I violate it sometimes, too.] Patch applied with the s/BUG/WARN bikeshed. -Daniel Thanks for applying the patch, it's certainly better than what we have currently. Why I wanted a BUG: When you get a ref to an object without holding a lock you get a potentially unsafe pointer (to which we will be writing). If the context object memory is freed, and we write to it, we have a potential to late scribble over insert your file system of choice memory. There is probably a similar security implication there as well. Many of us are used to, and capable of recovering from GPU hangs, but less of us like to deal with FS recovery. I actually believe all get code like this (backed with refcounts) should BUG and not WARN. -- Ben Widawsky, 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 02/12] drm/i915: BUG_ON bad PPGTT offset
On Mon, May 06, 2013 at 11:48:05AM +0200, Daniel Vetter wrote:
On Thu, May 02, 2013 at 01:28:23PM -0700, Jesse Barnes wrote:
On Tue, 23 Apr 2013 23:15:30 -0700
Ben Widawsky b...@bwidawsk.net wrote:
Because PPGTT PDEs within the GTT are calculated in cachelines
(HW guys consistency ftw) we do a divide which will wreak havoc if this
is wrong, and I know that from experience).
If/when we move to multiple PPGTTs this will have to become a WARN, and
return an error. For now however it should always be considered fatal,
and only a developer could hit it.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 50df194..0503f09 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -85,6 +85,8 @@ static int gen6_ppgtt_enable(struct drm_device *dev)
uint32_t pd_entry;
int i;
+ BUG_ON(ppgtt-pd_offset 0x3f);
+
pd_addr = (gen6_gtt_pte_t __iomem*)dev_priv-gtt.gsm +
ppgtt-pd_offset / sizeof(gen6_gtt_pte_t);
for (i = 0; i ppgtt-num_pd_entries; i++) {
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Queued for -next with the same s/BUG/WARN bikeshed, thanks for the patch.
-Daniel
If you're going to change to WARN, please fixup the patch to return
early... if (WARN_ON(...)) return
The GPU will end up hanging if we've messed this up...
--
Ben Widawsky, 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 01/12] drm/i915: Assert mutex_is_locked on context lookup
On Mon, May 6, 2013 at 7:59 PM, Ben Widawsky b...@bwidawsk.net wrote: Why I wanted a BUG: When you get a ref to an object without holding a lock you get a potentially unsafe pointer (to which we will be writing). If the context object memory is freed, and we write to it, we have a potential to late scribble over insert your file system of choice memory. There is probably a similar security implication there as well. Many of us are used to, and capable of recovering from GPU hangs, but less of us like to deal with FS recovery. I actually believe all get code like this (backed with refcounts) should BUG and not WARN. Historically we've screwed up locking in the driver load/teardown, suspend/resume and panic paths. Blowing up with a BUG_ON in there tends to be pretty bad for debugging. And there's pretty much no chance of a hostile party being able to exploit a race. So _especially_ for locking checks imo only WARN is acceptable. Otoh if such a race is indeed expoitable from userspace and it escaped into a released kernel that means we have a pretty gapping hole in our test coverage. Fixing that sounds more fruitful to me than making bug reporter's life harder. -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 02/12] drm/i915: BUG_ON bad PPGTT offset
On Mon, May 6, 2013 at 8:03 PM, Ben Widawsky b...@bwidawsk.net wrote:
On Mon, May 06, 2013 at 11:48:05AM +0200, Daniel Vetter wrote:
On Thu, May 02, 2013 at 01:28:23PM -0700, Jesse Barnes wrote:
On Tue, 23 Apr 2013 23:15:30 -0700
Ben Widawsky b...@bwidawsk.net wrote:
Because PPGTT PDEs within the GTT are calculated in cachelines
(HW guys consistency ftw) we do a divide which will wreak havoc if this
is wrong, and I know that from experience).
If/when we move to multiple PPGTTs this will have to become a WARN, and
return an error. For now however it should always be considered fatal,
and only a developer could hit it.
Signed-off-by: Ben Widawsky b...@bwidawsk.net
---
drivers/gpu/drm/i915/i915_gem_gtt.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 50df194..0503f09 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -85,6 +85,8 @@ static int gen6_ppgtt_enable(struct drm_device *dev)
uint32_t pd_entry;
int i;
+ BUG_ON(ppgtt-pd_offset 0x3f);
+
pd_addr = (gen6_gtt_pte_t __iomem*)dev_priv-gtt.gsm +
ppgtt-pd_offset / sizeof(gen6_gtt_pte_t);
for (i = 0; i ppgtt-num_pd_entries; i++) {
Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org
Queued for -next with the same s/BUG/WARN bikeshed, thanks for the patch.
-Daniel
If you're going to change to WARN, please fixup the patch to return
early... if (WARN_ON(...)) return
The GPU will end up hanging if we've messed this up...
I don't care about the gpu, only that the logs hit the disc. And I
don't see how an early return can safe anything in here, since
something went horribly wrong already anyway.
-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: replace snb_update_wm with haswell_update_wm on HSW
From: Paulo Zanoni paulo.r.zan...@intel.com
We were previously calling sandybridge_update_wm on HSW, but the SNB
function didn't really match the HSW specification, so we were just
writing the wrong values. For example, I was not seeing any LP
watermark ever enabled.
So this patch implements the haswell_update_wm as described in our
specification. The values match the Haswell Watermark Calculator
tool. There are only 2 pieces missing for this code:
1 - Sprite watermarks. We currently set the maximum possible value
for the WM_PIPE register and we never enable LP watermarks if any
sprite is enabled, so we should be safe from bugs. A future patch
should add support for correctly setting the sprite WMs.
2 - Support for 5/6 data buffer partitioning. This is only useful if
we have sprite watermarks enabled and working, and should be simple
to implement with the infrastructure provided by this patch.
With this patch I can finally see us reaching PC7 state with screen
sizes = 1920x1080.
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
drivers/gpu/drm/i915/i915_reg.h |3 +
drivers/gpu/drm/i915/intel_pm.c | 411 +--
2 files changed, 401 insertions(+), 13 deletions(-)
This patch applies on top of dinq + all the 15 patches I previously sent to
the intel-gfx mailing list.
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c05bd4a..1aed704 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4924,6 +4924,9 @@
#define SFUSE_STRAP_DDIC_DETECTED (11)
#define SFUSE_STRAP_DDID_DETECTED (10)
+#define WM_MISC0x45260
+#define WM_MISC_DATA_PARTITION_5_6(1 0)
+
#define WM_DBG 0x45280
#define WM_DBG_DISALLOW_MULTIPLE_LP (10)
#define WM_DBG_DISALLOW_MAXFIFO (11)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e58f47c..707d0a4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2014,20 +2014,189 @@ static void ivybridge_update_wm(struct drm_device *dev)
cursor_wm);
}
-static void
-haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc)
+static int hsw_wm_get_pixel_rate(struct drm_device *dev,
+struct drm_crtc *crtc)
+{
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ int pixel_rate, pfit_size;
+
+ if (intel_crtc-config.pixel_target_clock)
+ pixel_rate = intel_crtc-config.pixel_target_clock;
+ else
+ pixel_rate = intel_crtc-config.adjusted_mode.clock;
+
+ /* We only use IF-ID interlacing. If we ever use PF-ID we'll need to
+* adjust the pixel_rate here. */
+
+ pfit_size = intel_crtc-config.pch_pfit.size;
+ if (pfit_size) {
+ int x, y, crtc_x, crtc_y, hscale, vscale, totscale;
+
+ x = (pfit_size 16) 0x;
+ y = pfit_size 0x;
+ crtc_x = intel_crtc-config.adjusted_mode.hdisplay;
+ crtc_y = intel_crtc-config.adjusted_mode.vdisplay;
+
+ hscale = crtc_x * 1000 / x;
+ vscale = crtc_y * 1000 / y;
+ hscale = (hscale 1000) ? 1000 : hscale;
+ vscale = (vscale 1000) ? 1000 : vscale;
+ totscale = hscale * vscale / 1000;
+ pixel_rate = pixel_rate * totscale / 1000;
+ }
+
+ return pixel_rate;
+}
+
+static int hsw_wm_method1(int pixel_rate, int bytes_per_pixel, int latency)
+{
+ int ret;
+
+ ret = pixel_rate * bytes_per_pixel * latency;
+ ret = DIV_ROUND_UP(ret, 64 * 1) + 2;
+ return ret;
+}
+
+static int hsw_wm_method2(int pixel_rate, int pipe_htotal, int horiz_pixels,
+ int bytes_per_pixel, int latency)
+{
+ int ret;
+
+ ret = DIV_ROUND_UP(pipe_htotal * 1000, pixel_rate);
+ ret = ((latency / (ret * 10)) + 1) * horiz_pixels * bytes_per_pixel;
+ ret = DIV_ROUND_UP(ret, 64) + 2;
+ return ret;
+}
+
+static int hsw_wm_fbc(int pri_val, int horiz_pixels, int bytes_per_pixel)
+{
+ return DIV_ROUND_UP(pri_val * 64, horiz_pixels * bytes_per_pixel) + 2;
+}
+
+struct hsw_pipe_wm_parameters {
+ bool active;
+ bool sprite_enabled;
+ int pipe_htotal;
+ int pixel_rate;
+ int pri_bytes_per_pixel;
+ int cur_bytes_per_pixel;
+ int pri_horiz_pixels;
+ int cur_horiz_pixels;
+};
+
+struct hsw_lp_wm_result {
+ bool enable;
+ bool fbc_enable;
+ int pri_val;
+ int cur_val;
+ int fbc_val;
+};
+
+static void hsw_compute_lp_wm(int latency, int pri_max, int cur_max,
+ int fbc_max,
+ struct hsw_pipe_wm_parameters *params,
+ struct hsw_lp_wm_result *result)
+{
+ enum pipe pipe;
+ int pri_val[3],
Re: [Intel-gfx] [PATCH 3/4] drm/i915: set proper DPIO post divider for VGA on VLV v4
On Mon, May 06, 2013 at 10:52:36AM -0700, Kenneth Graunke wrote: On 05/02/2013 10:48 AM, Jesse Barnes wrote: Supposedly we should use the DAC divider for 300MHz pixel clocks, but as that doesn't actually work as well as the high freq divider here in practice, just use the high freq divider all the time. v2: remove unconditional write (Jesse) check for pixel rate properly (Jesse) v3: give up, the DAC divider apparently doesn't work, and low res modes work ok (Jesse) remove debug msg (Jesse) Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org VGA is broken with today's drm-intel-next-queued (3a2128bcac1ae). Applying this patch fixes it. Thanks! Tested-by: Kenneth Graunke kenn...@whitecape.org Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/6] drm/i915: Add support for FBC on Ivybridge.
This patch introduce Frame Buffer Compression (FBC) support for IVB,
without enabling it by default.
It adds a new function gen7_enable_fbc to avoid getting
ironlake_enable_fbc messed with many IS_IVYBRIDGE checks.
v2: Fixes from Ville.
* Fix Plane. FBC is tied to primary plane A in HSW
* Fix DPFC initial write to avoid let trash on the register.
v3: Checking for bad plane on intel_update_fbc() as Chris suggested.
v4: Ville pointed out that according to BSpec FBC_CTL bits 0:3 must be 0.
v5: Up to v4 this work was entirely focused on Haswell. However Ville
noticed I could reuse the FBC work done for HSW and get FBC for free
at Ivybridge. So it makes more sense enable FBC for IVB first.
FBC for HSW comming on next patches. We are just not enabling it by
default on IVB.
v6: Fix confused commit name (by Matt Turner).
v7: Remove gtt_offset shift since it is page aligned byte offset (by Ville).
Cc: Matt Turner matts...@gmail.com
Cc: 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
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/i915_drv.c | 1 +
drivers/gpu/drm/i915/i915_reg.h | 6 ++
drivers/gpu/drm/i915/intel_pm.c | 33 +++--
3 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 624cdfc..319dc83 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -280,6 +280,7 @@ static const struct intel_device_info
intel_ivybridge_m_info = {
GEN7_FEATURES,
.is_ivybridge = 1,
.is_mobile = 1,
+ .has_fbc = 1,
};
static const struct intel_device_info intel_ivybridge_q_info = {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a470103..a817b79 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -924,7 +924,9 @@
#define DPFC_CTL_EN (131)
#define DPFC_CTL_PLANEA (030)
#define DPFC_CTL_PLANEB (130)
+#define IVB_DPFC_CTL_PLANE_SHIFT (29)
#define DPFC_CTL_FENCE_EN(129)
+#define IVB_DPFC_CTL_FENCE_EN(128)
#define DPFC_CTL_PERSISTENT_MODE (125)
#define DPFC_SR_EN (110)
#define DPFC_CTL_LIMIT_1X(06)
@@ -957,6 +959,7 @@
#define ILK_DPFC_CHICKEN 0x43224
#define ILK_FBC_RT_BASE0x2128
#define ILK_FBC_RT_VALID (10)
+#define SNB_FBC_FRONT_BUFFER (11)
#define ILK_DISPLAY_CHICKEN1 0x42000
#define ILK_FBCQ_DIS (122)
@@ -972,6 +975,9 @@
#define SNB_CPU_FENCE_ENABLE (129)
#define DPFC_CPU_FENCE_OFFSET 0x100104
+/* Framebuffer compression for Ivybridge */
+#define IVB_FBC_RT_BASE0x7020
+
/*
* GPIO regs
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 556b989..5efa283 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -253,6 +253,30 @@ static bool ironlake_fbc_enabled(struct drm_device *dev)
return I915_READ(ILK_DPFC_CONTROL) DPFC_CTL_EN;
}
+static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval)
+{
+ struct drm_device *dev = crtc-dev;
+ struct drm_i915_private *dev_priv = dev-dev_private;
+ struct drm_framebuffer *fb = crtc-fb;
+ struct intel_framebuffer *intel_fb = to_intel_framebuffer(fb);
+ struct drm_i915_gem_object *obj = intel_fb-obj;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+ I915_WRITE(IVB_FBC_RT_BASE, obj-gtt_offset | ILK_FBC_RT_VALID);
+
+ I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X |
+ IVB_DPFC_CTL_FENCE_EN |
+ intel_crtc-plane IVB_DPFC_CTL_PLANE_SHIFT);
+
+ I915_WRITE(SNB_DPFC_CTL_SA,
+ SNB_CPU_FENCE_ENABLE | obj-fence_reg);
+ I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc-y);
+
+ sandybridge_blit_fbc_update(dev);
+
+ DRM_DEBUG_KMS(enabled fbc on plane %d\n, intel_crtc-plane);
+}
+
bool intel_fbc_enabled(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -439,7 +463,7 @@ void intel_update_fbc(struct drm_device *dev)
if (enable_fbc 0) {
DRM_DEBUG_KMS(fbc set to per-chip default\n);
enable_fbc = 1;
- if (INTEL_INFO(dev)-gen = 6)
+ if (INTEL_INFO(dev)-gen = 7)
enable_fbc = 0;
}
if (!enable_fbc) {
@@ -4419,7 +4443,12 @@ void intel_init_pm(struct drm_device *dev)
if (I915_HAS_FBC(dev)) {
if (HAS_PCH_SPLIT(dev)) {
dev_priv-display.fbc_enabled = ironlake_fbc_enabled;
- dev_priv-display.enable_fbc = ironlake_enable_fbc;
+ if (IS_IVYBRIDGE(dev))
+
[Intel-gfx] [PATCH 2/6] drm/i915: IVB FBC WaFbcAsynchFlipDisableFbcQueue
Display register 42000h bit 22 must be set to 1b for the entire time that Frame Buffer Compression is enabled. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com --- drivers/gpu/drm/i915/intel_pm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 5efa283..4661f9f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -268,6 +268,8 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval) IVB_DPFC_CTL_FENCE_EN | intel_crtc-plane IVB_DPFC_CTL_PLANE_SHIFT); + /* WaFbcAsynchFlipDisableFbcQueue */ + I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS); I915_WRITE(SNB_DPFC_CTL_SA, SNB_CPU_FENCE_ENABLE | obj-fence_reg); I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc-y); -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] drm/i915: IVB FBC WaFbcDisableDpfcClockGating
Display register 42020h bit 9 must be set to 1b for the entire time that Frame Buffer Compression is enabled. v2: RMW to preserve other bits (by Ville) v3: Fix from Ville: sed /| at RMW Cc: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com --- drivers/gpu/drm/i915/intel_pm.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 4661f9f..e9fb0ba 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -242,6 +242,12 @@ static void ironlake_disable_fbc(struct drm_device *dev) dpfc_ctl = ~DPFC_CTL_EN; I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl); + if (IS_IVYBRIDGE(dev)) + /* WaFbcDisableDpfcClockGating */ + I915_WRITE(ILK_DSPCLK_GATE_D, + I915_READ(ILK_DSPCLK_GATE_D) | + ~ILK_DPFCUNIT_CLOCK_GATE_DISABLE); + DRM_DEBUG_KMS(disabled FBC\n); } } @@ -270,6 +276,11 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval) /* WaFbcAsynchFlipDisableFbcQueue */ I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS); + /* WaFbcDisableDpfcClockGating */ + I915_WRITE(ILK_DSPCLK_GATE_D, + I915_READ(ILK_DSPCLK_GATE_D) | + ILK_DPFCUNIT_CLOCK_GATE_DISABLE); + I915_WRITE(SNB_DPFC_CTL_SA, SNB_CPU_FENCE_ENABLE | obj-fence_reg); I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc-y); -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] drm/i915: Enable FBC at Haswell.
This patch introduce Frame Buffer Compression (FBC) support for HSW.
FBC is tied to primary plane A in HSW.
v2: Ville pointed out docs say FBC must be disabled before disabling
the plane on HSW.
v3: Really enabling it by default at HSW.
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/i915_drv.c | 1 +
drivers/gpu/drm/i915/intel_display.c | 5 +++--
drivers/gpu/drm/i915/intel_pm.c | 21 -
3 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 319dc83..f11708a 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -319,6 +319,7 @@ static const struct intel_device_info intel_haswell_m_info
= {
.is_mobile = 1,
.has_ddi = 1,
.has_fpga_dbg = 1,
+ .has_fbc = 1,
};
static const struct pci_device_id pciidlist[] = { /* aka */
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5491a58..8e5966e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3518,11 +3518,12 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
drm_vblank_off(dev, pipe);
intel_crtc_update_cursor(crtc, false);
- intel_disable_plane(dev_priv, plane, pipe);
-
+ /* FBC must be disabled before disabling the plane on HSW. */
if (dev_priv-cfb_plane == plane)
intel_disable_fbc(dev);
+ intel_disable_plane(dev_priv, plane, pipe);
+
if (intel_crtc-config.has_pch_encoder)
intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, false);
intel_disable_pipe(dev_priv, pipe);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e9fb0ba..5d40799 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -274,12 +274,14 @@ static void gen7_enable_fbc(struct drm_crtc *crtc,
unsigned long interval)
IVB_DPFC_CTL_FENCE_EN |
intel_crtc-plane IVB_DPFC_CTL_PLANE_SHIFT);
- /* WaFbcAsynchFlipDisableFbcQueue */
- I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
- /* WaFbcDisableDpfcClockGating */
- I915_WRITE(ILK_DSPCLK_GATE_D,
- I915_READ(ILK_DSPCLK_GATE_D) |
- ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
+ if (IS_IVYBRIDGE(dev)) {
+ /* WaFbcAsynchFlipDisableFbcQueue */
+ I915_WRITE(ILK_DISPLAY_CHICKEN1, ILK_FBCQ_DIS);
+ /* WaFbcDisableDpfcClockGating */
+ I915_WRITE(ILK_DSPCLK_GATE_D,
+ I915_READ(ILK_DSPCLK_GATE_D) |
+ ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
+ }
I915_WRITE(SNB_DPFC_CTL_SA,
SNB_CPU_FENCE_ENABLE | obj-fence_reg);
@@ -476,7 +478,7 @@ void intel_update_fbc(struct drm_device *dev)
if (enable_fbc 0) {
DRM_DEBUG_KMS(fbc set to per-chip default\n);
enable_fbc = 1;
- if (INTEL_INFO(dev)-gen = 7)
+ if (INTEL_INFO(dev)-gen = 7 !IS_HASWELL(dev))
enable_fbc = 0;
}
if (!enable_fbc) {
@@ -497,7 +499,8 @@ void intel_update_fbc(struct drm_device *dev)
dev_priv-no_fbc_reason = FBC_MODE_TOO_LARGE;
goto out_disable;
}
- if ((IS_I915GM(dev) || IS_I945GM(dev)) intel_crtc-plane != 0) {
+ if ((IS_I915GM(dev) || IS_I945GM(dev) || IS_HASWELL(dev))
+ intel_crtc-plane != 0) {
DRM_DEBUG_KMS(plane not 0, disabling compression\n);
dev_priv-no_fbc_reason = FBC_BAD_PLANE;
goto out_disable;
@@ -4456,7 +4459,7 @@ void intel_init_pm(struct drm_device *dev)
if (I915_HAS_FBC(dev)) {
if (HAS_PCH_SPLIT(dev)) {
dev_priv-display.fbc_enabled = ironlake_fbc_enabled;
- if (IS_IVYBRIDGE(dev))
+ if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
dev_priv-display.enable_fbc =
gen7_enable_fbc;
else
--
1.7.11.7
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/6] drm/i915: HSW FBC WaFbcAsynchFlipDisableFbcQueue
Display register 420B0h bit 22 must be set to 1b for the entire time that
Frame Buffer Compression is enabled.
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com
---
drivers/gpu/drm/i915/i915_reg.h | 7 +++
drivers/gpu/drm/i915/intel_pm.c | 4
2 files changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a817b79..a17480e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -979,6 +979,13 @@
#define IVB_FBC_RT_BASE0x7020
+#define _HSW_PIPE_SLICE_CHICKEN_1_A0x420B0
+#define _HSW_PIPE_SLICE_CHICKEN_1_B0x420B4
+#define HSW_BYPASS_FBC_QUEUE (122)
+#define HSW_PIPE_SLICE_CHICKEN_1(pipe) _PIPE(pipe, + \
+_HSW_PIPE_SLICE_CHICKEN_1_A, + \
+_HSW_PIPE_SLICE_CHICKEN_1_B)
+
/*
* GPIO regs
*/
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5d40799..f074c0c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -281,6 +281,10 @@ static void gen7_enable_fbc(struct drm_crtc *crtc,
unsigned long interval)
I915_WRITE(ILK_DSPCLK_GATE_D,
I915_READ(ILK_DSPCLK_GATE_D) |
ILK_DPFCUNIT_CLOCK_GATE_DISABLE);
+ } else {
+ /* WaFbcAsynchFlipDisableFbcQueue */
+ I915_WRITE(HSW_PIPE_SLICE_CHICKEN_1(intel_crtc-pipe),
+ HSW_BYPASS_FBC_QUEUE);
}
I915_WRITE(SNB_DPFC_CTL_SA,
--
1.7.11.7
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] drm/i915: HSW FBC WaFbcDisableDpfcClockGating
Display register 46500h bit 23 must be set to 1b for the entire time that Frame Buffer Compression is enabled. v2: Ville suggested to enable it back when disabling fbc to avoid wasting power. v3: RMW to preserve other bits (by Ville) v4: Fix from Ville: sed /| at RMW Cc: Ville Syrjälä ville.syrj...@linux.intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 10 ++ 2 files changed, 13 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index a17480e..40a59e5 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -986,6 +986,9 @@ _HSW_PIPE_SLICE_CHICKEN_1_A, + \ _HSW_PIPE_SLICE_CHICKEN_1_B) +#define HSW_CLKGATE_DISABLE_PART_1 0x46500 +#define HSW_DPFC_GATING_DISABLE (123) + /* * GPIO regs */ diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index f074c0c..9b5a38c 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -248,6 +248,12 @@ static void ironlake_disable_fbc(struct drm_device *dev) I915_READ(ILK_DSPCLK_GATE_D) | ~ILK_DPFCUNIT_CLOCK_GATE_DISABLE); + if(IS_HASWELL(dev)) + /* WaFbcDisableDpfcClockGating */ + I915_WRITE(HSW_CLKGATE_DISABLE_PART_1, + I915_READ(HSW_CLKGATE_DISABLE_PART_1) | + ~HSW_DPFC_GATING_DISABLE); + DRM_DEBUG_KMS(disabled FBC\n); } } @@ -285,6 +291,10 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval) /* WaFbcAsynchFlipDisableFbcQueue */ I915_WRITE(HSW_PIPE_SLICE_CHICKEN_1(intel_crtc-pipe), HSW_BYPASS_FBC_QUEUE); + /* WaFbcDisableDpfcClockGating */ + I915_WRITE(HSW_CLKGATE_DISABLE_PART_1, + I915_READ(HSW_CLKGATE_DISABLE_PART_1) | + HSW_DPFC_GATING_DISABLE); } I915_WRITE(SNB_DPFC_CTL_SA, -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] linux-next: manual merge of the drm-intel tree with Linus' tree
Hi all, Today's linux-next merge of the drm-intel tree got a conflict in drivers/gpu/drm/i915/i915_reg.h between commit a65851af5938 (drm/i915: Make data/link N value power of two) from Linus' tree and commit e3b95f1eb5b9 (drm/i915: Apply OCD to data/link m/n register #defines) from the drm-intel tree. I fixed it up (see below) and can carry the fix as necessary (no action is required). Daniel, I assume all this stuff being added to the drm-intel tree is going upstream very soon? -- Cheers, Stephen Rothwells...@canb.auug.org.au diff --cc drivers/gpu/drm/i915/i915_reg.h index 83f9c26,a470103..000 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@@ -2648,18 -2768,19 +2768,19 @@@ * which is after the LUTs, so we want the bytes for our color format. * For our current usage, this is always 3, one byte for R, G and B. */ - #define _PIPEA_GMCH_DATA_M0x70050 - #define _PIPEB_GMCH_DATA_M0x71050 + #define _PIPEA_DATA_M_G4X 0x70050 + #define _PIPEB_DATA_M_G4X 0x71050 /* Transfer unit size for display port - 1, default is 0x3f (for TU size 64) */ -#define PIPE_GMCH_DATA_M_TU_SIZE_MASK (0x3f 25) -#define PIPE_GMCH_DATA_M_TU_SIZE_SHIFT 25 +#define TU_SIZE(x) (((x)-1) 25) /* default size 64 */ +#define TU_SIZE_MASK (0x3f 25) + #define TU_SIZE_SHIFT25 -#define PIPE_GMCH_DATA_M_MASK (0xff) +#define DATA_LINK_M_N_MASK (0xff) +#define DATA_LINK_N_MAX (0x80) - #define _PIPEA_GMCH_DATA_N0x70054 - #define _PIPEB_GMCH_DATA_N0x71054 + #define _PIPEA_DATA_N_G4X 0x70054 + #define _PIPEB_DATA_N_G4X 0x71054 -#define PIPE_GMCH_DATA_N_MASK (0xff) /* * Computing Link M and N values for the Display Port link @@@ -2672,16 -2793,18 +2793,16 @@@ * Attributes and VB-ID. */ - #define _PIPEA_DP_LINK_M 0x70060 - #define _PIPEB_DP_LINK_M 0x71060 + #define _PIPEA_LINK_M_G4X 0x70060 + #define _PIPEB_LINK_M_G4X 0x71060 -#define PIPEA_DP_LINK_M_MASK(0xff) - #define _PIPEA_DP_LINK_N 0x70064 - #define _PIPEB_DP_LINK_N 0x71064 + #define _PIPEA_LINK_N_G4X 0x70064 + #define _PIPEB_LINK_N_G4X 0x71064 -#define PIPEA_DP_LINK_N_MASK(0xff) - #define PIPE_GMCH_DATA_M(pipe) _PIPE(pipe, _PIPEA_GMCH_DATA_M, _PIPEB_GMCH_DATA_M) - #define PIPE_GMCH_DATA_N(pipe) _PIPE(pipe, _PIPEA_GMCH_DATA_N, _PIPEB_GMCH_DATA_N) - #define PIPE_DP_LINK_M(pipe) _PIPE(pipe, _PIPEA_DP_LINK_M, _PIPEB_DP_LINK_M) - #define PIPE_DP_LINK_N(pipe) _PIPE(pipe, _PIPEA_DP_LINK_N, _PIPEB_DP_LINK_N) + #define PIPE_DATA_M_G4X(pipe) _PIPE(pipe, _PIPEA_DATA_M_G4X, _PIPEB_DATA_M_G4X) + #define PIPE_DATA_N_G4X(pipe) _PIPE(pipe, _PIPEA_DATA_N_G4X, _PIPEB_DATA_N_G4X) + #define PIPE_LINK_M_G4X(pipe) _PIPE(pipe, _PIPEA_LINK_M_G4X, _PIPEB_LINK_M_G4X) + #define PIPE_LINK_N_G4X(pipe) _PIPE(pipe, _PIPEA_LINK_N_G4X, _PIPEB_LINK_N_G4X) /* Display cursor control */ pgpwIJS5sxxNH.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
