Re: [Intel-gfx] [PATCH] drm/i915: Use RCS flips on Ivybridge+
On Tue, Aug 20, 2013 at 1:34 AM, Chris Wilson wrote:
> RCS flips do work on Iybridge+ so long as we can unmask the messages
> through DERRMR. However, there are quite a few workarounds mentioned
> regarding unmasking more than one event or triggering more than one
> message through DERRMR. Those workarounds in principle prevent us from
> performing pipelined flips (and asynchronous flips across multiple
> planes) and equally apply to the "known good" BCS ring. Given that it
> already appears to work, and also appears to work with unmasking all 3
> planes at once (and queuing flips across multiple planes), be brave.
>
> Bugzlla: https://bugs.freedesktop.org/show_bug.cgi?id=67600
> Signed-off-by: Chris Wilson
Seems to work great here. Tested-by: Stéphane Marchesin
> ---
> drivers/gpu/drm/i915/i915_reg.h | 17 +
> drivers/gpu/drm/i915/intel_display.c | 45
> +++---
> 2 files changed, 53 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e2690ec..730510d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -679,6 +679,23 @@
> #define FPGA_DBG_RM_NOCLAIM (1<<31)
>
> #define DERRMR 0x44050
> +#define DERRMR_PIPEA_SCANLINE(1<<0)
> +#define DERRMR_PIPEA_PRI_FLIP_DONE (1<<1)
> +#define DERRMR_PIPEA_SPR_FLIP_DONE (1<<2)
> +#define DERRMR_PIPEA_VBLANK (1<<3)
> +#define DERRMR_PIPEA_HBLANK (1<<5)
> +#define DERRMR_PIPEB_SCANLINE(1<<8)
> +#define DERRMR_PIPEB_PRI_FLIP_DONE (1<<9)
> +#define DERRMR_PIPEB_SPR_FLIP_DONE (1<<10)
> +#define DERRMR_PIPEB_VBLANK (1<<11)
> +#define DERRMR_PIPEB_HBLANK (1<<13)
> +/* Note that PIPEC is not a simple translation of PIPEA/PIPEB */
> +#define DERRMR_PIPEC_SCANLINE(1<<14)
> +#define DERRMR_PIPEC_PRI_FLIP_DONE (1<<15)
> +#define DERRMR_PIPEC_SPR_FLIP_DONE (1<<20)
> +#define DERRMR_PIPEC_VBLANK (1<<21)
> +#define DERRMR_PIPEC_HBLANK (1<<22)
> +
>
> /* GM45+ chicken bits -- debug workaround bits that may be required
> * for various sorts of correct behavior. The top 16 bits of each are
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 727a123..55c9b39 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7653,12 +7653,6 @@ err:
> return ret;
> }
>
> -/*
> - * On gen7 we currently use the blit ring because (in early silicon at least)
> - * the render ring doesn't give us interrpts for page flip completion, which
> - * means clients will hang after the first flip is queued. Fortunately the
> - * blit ring generates interrupts properly, so use it instead.
> - */
> static int intel_gen7_queue_flip(struct drm_device *dev,
> struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> @@ -7666,9 +7660,13 @@ static int intel_gen7_queue_flip(struct drm_device
> *dev,
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> - struct intel_ring_buffer *ring = &dev_priv->ring[BCS];
> + struct intel_ring_buffer *ring;
> uint32_t plane_bit = 0;
> - int ret;
> + int len, ret;
> +
> + ring = obj->ring;
> + if (ring == NULL || ring->id != RCS)
> + ring = &dev_priv->ring[BCS];
>
> ret = intel_pin_and_fence_fb_obj(dev, obj, ring);
> if (ret)
> @@ -7690,10 +7688,39 @@ static int intel_gen7_queue_flip(struct drm_device
> *dev,
> goto err_unpin;
> }
>
> - ret = intel_ring_begin(ring, 4);
> + len = 4;
> + if (ring->id == RCS)
> + len += 6;
> +
> + ret = intel_ring_begin(ring, len);
> if (ret)
> goto err_unpin;
>
> + /* Unmask the flip-done completion message. Note that the bspec says
> that
> +* we should do this for both the BCS and RCS, and that we must not
> unmask
> +* more than one flip event at any time (or ensure that one flip
> message
> +* can be sent by waiting for flip-done prior to queueing new flips).
> +* Experimentation says that BCS works despite DERRMR masking all
> +* flip-done completion events and that unmasking all planes at once
> +* for the RCS also doesn't appear to drop events. Setting the DERRMR
> +* to zero does lead to lockups within MI_DISPLAY_FLIP.
> +*/
> + if (ring->id == RCS) {
> + struct { /* XXX This is quite rude! */
> + struct drm_i915_gem_object *scratch;
> + } *priv = ring->private;
> + u32 addr = i915_gem_obj_ggtt_offset(priv->scratch) + 128;
> +
> + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
>
Re: [Intel-gfx] [PATCH] drm/i915: Restore the preliminary HW check.
On Fri, 23 Aug 2013 16:00:07 -0700 Ben Widawsky wrote: > We still maintain code internally that cares about preliminary support. > Leaving the check here doesn't hurt anyone, and should keep things more > in line. > > This time around, stick the info in the intel_info structure, and also > change the error from DRM_ERROR->DRM_INFO. I hate this param. But this looks like a better way to do it than the ID list we had in pci_probe before. Reviewed-by: Jesse Barnes -- Jesse Barnes, 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: Restore the preliminary HW check.
We still maintain code internally that cares about preliminary support.
Leaving the check here doesn't hurt anyone, and should keep things more
in line.
This time around, stick the info in the intel_info structure, and also
change the error from DRM_ERROR->DRM_INFO.
This is a partial revert of:
commit 590e4df8c82e6c2707ae12ba6672ab6fb9cd4b89
Author: Jesse Barnes
Date: Wed May 8 10:45:15 2013 -0700
drm/i915: VLV support is no longer preliminary
Daniel, I'll provide the fix ups for internal too if/when you merge
this (if you want).
Cc: Jesse Barnes
Signed-off-by: Ben Widawsky
---
drivers/gpu/drm/i915/i915_drv.c | 6 ++
drivers/gpu/drm/i915/i915_drv.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 735dd56..ef7425c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -895,6 +895,12 @@ static int i915_pci_probe(struct pci_dev *pdev, const
struct pci_device_id *ent)
struct intel_device_info *intel_info =
(struct intel_device_info *) ent->driver_data;
+ if (IS_PRELIMINARY_HW(intel_info) && !i915_preliminary_hw_support) {
+ DRM_INFO("This hardware requires preliminary hardware
support.\n"
+"See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or
modparam preliminary_hw_support\n");
+ return -ENODEV;
+ }
+
/* Only bind to function 0 of the device. Early generations
* used function 1 as a placeholder for multi-head. This causes
* us confusion instead, especially on the systems where both
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8ee15b4..78cd6f9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -419,6 +419,7 @@ struct intel_uncore {
func(is_ivybridge) sep \
func(is_valleyview) sep \
func(is_haswell) sep \
+ func(is_preliminary) sep \
func(has_force_wake) sep \
func(has_fbc) sep \
func(has_pipe_cxsr) sep \
@@ -1592,6 +1593,7 @@ struct drm_i915_file_private {
((dev)->pci_device & 0xFF00) == 0x0C00)
#define IS_ULT(dev)(IS_HASWELL(dev) && \
((dev)->pci_device & 0xFF00) == 0x0A00)
+#define IS_PRELIMINARY_HW(intel_info) ((intel_info)->is_preliminary)
/*
* The genX designation typically refers to the render engine, so render
--
1.8.3.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: enable trickle feed on Haswell
From: Paulo Zanoni
We shouldn't disable the trickle feed bits on Haswell. Our
documentation explicitly says the trickle feed bits of PRI_CTL and
CUR_CTL should not be programmed to 1, and the hardware engineer also
asked us to not program the SPR_CTL field to 1. Leaving the bits as 1
could cause underflows.
Reported-by: Arthur Runyan
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_reg.h | 1 +
drivers/gpu/drm/i915/intel_display.c | 10 +++---
drivers/gpu/drm/i915/intel_pm.c | 2 --
drivers/gpu/drm/i915/intel_sprite.c | 7 +--
4 files changed, 13 insertions(+), 7 deletions(-)
Some side discussions:
1 - It seems we always do read-modify-write with the PRI/SPR/CUR registers. I
think this is dangerous since we may forget to disable something and keep our
code in a bugged state forever. Shouldn't we try to completely set the full
state of the bits from scratch every time we enable PRI/SPR/CUR?
2 - We also have code to enable the trickle feed bits at init_clock_gating.
Isn't this dangerous? Trickle fee changes the memory is read, my first guess
would be that it's probably not safe to change this bit when things are
enabled. Also, we lose the state of these bits when the power well is disabled,
so the code in init_clock_gating really makes no sense on Haswell.
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index bbbc177..e92bb47 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3312,6 +3312,7 @@
#define MCURSOR_PIPE_A 0x00
#define MCURSOR_PIPE_B (1 << 28)
#define MCURSOR_GAMMA_ENABLE (1 << 26)
+#define CURSOR_TRICKLE_FEED_DISABLE (1 << 14)
#define _CURABASE (dev_priv->info->display_mmio_offset + 0x70084)
#define _CURAPOS (dev_priv->info->display_mmio_offset + 0x70088)
#define CURSOR_POS_MASK 0x007FF
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index c64631d..d5f038e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2077,8 +2077,10 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
else
dspcntr &= ~DISPPLANE_TILED;
- /* must disable */
- dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
+ if (IS_HASWELL(dev))
+ dspcntr &= ~DISPPLANE_TRICKLE_FEED_DISABLE;
+ else
+ dspcntr |= DISPPLANE_TRICKLE_FEED_DISABLE;
I915_WRITE(reg, dspcntr);
@@ -6768,8 +6770,10 @@ static void ivb_update_cursor(struct drm_crtc *crtc, u32
base)
cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
cntl |= CURSOR_MODE_DISABLE;
}
- if (IS_HASWELL(dev))
+ if (IS_HASWELL(dev)) {
cntl |= CURSOR_PIPE_CSC_ENABLE;
+ cntl &= ~CURSOR_TRICKLE_FEED_DISABLE;
+ }
I915_WRITE(CURCNTR_IVB(pipe), cntl);
intel_crtc->cursor_visible = visible;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 4605682..3a5c0bb 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4950,8 +4950,6 @@ static void haswell_init_clock_gating(struct drm_device
*dev)
I915_READ(GEN7_SQ_CHICKEN_MBCUNIT_CONFIG) |
GEN7_SQ_CHICKEN_MBCUNIT_SQINTMOB);
- g4x_disable_trickle_feed(dev);
-
/* WaVSRefCountFullforceMissDisable:hsw */
gen7_setup_fixed_func_scheduler(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c
b/drivers/gpu/drm/i915/intel_sprite.c
index 78b621c..ad6ec4b 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -260,8 +260,11 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc
*crtc,
if (obj->tiling_mode != I915_TILING_NONE)
sprctl |= SPRITE_TILED;
- /* must disable */
- sprctl |= SPRITE_TRICKLE_FEED_DISABLE;
+ if (IS_HASWELL(dev))
+ sprctl &= ~SPRITE_TRICKLE_FEED_DISABLE;
+ else
+ sprctl |= SPRITE_TRICKLE_FEED_DISABLE;
+
sprctl |= SPRITE_ENABLE;
if (IS_HASWELL(dev))
--
1.8.1.2
___
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 mask EI UP interrupt on IVB|SNB
On Fri, Aug 23, 2013 at 10:18:39PM +, Azad, Vinit wrote: > Thanks Mika. It looks good to me. > > Reviewed-by: Vinit Azad > -Original Message- > From: Mika Kuoppala [mailto:mika.kuopp...@linux.intel.com] > Sent: Thursday, August 22, 2013 11:09 > To: intel-gfx@lists.freedesktop.org > Cc: Mika Kuoppala; Azad, Vinit > Subject: [PATCH] drm/i915: Don't mask EI UP interrupt on IVB|SNB > > Submitting a batchbuffer which simulates a gpu hang by doing > MI_BATCH_BUFFER_START into itself, to test hangcheck, started to hard hang > the whole box (IVB). Bisecting lead to this commit: > > commit 664b422c2966cd39b8f67e8d53a566ea8c877cd6 > Author: Vinit Azad > Date: Wed Aug 14 13:34:33 2013 -0700 > > drm/i915: Only unmask required PM interrupts > > Experimenting with the mask register showed that unmasking EI UP will prevent > the hard hang in IVB and SNB. > HSW doesn't hang with EI UP masked. > > Considering we are just disabling interrupts that aren't even delivered to > driver, this change is more likely to paper over some weirdness in gpu's > internal state machine. But until better explanation can be found, let's > trade little bit of power for stability on these architectures. > > v2: - Unmask EI_EXPIRED directly in I915_WRITE (Vinit) > v3: - Only unmask on SNB and IVB > > Cc: Vinit Azad > Signed-off-by: Mika Kuoppala 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] drm/i915: Don't mask EI UP interrupt on IVB|SNB
Thanks Mika. It looks good to me.
Reviewed-by: Vinit Azad
-Original Message-
From: Mika Kuoppala [mailto:mika.kuopp...@linux.intel.com]
Sent: Thursday, August 22, 2013 11:09
To: intel-gfx@lists.freedesktop.org
Cc: Mika Kuoppala; Azad, Vinit
Subject: [PATCH] drm/i915: Don't mask EI UP interrupt on IVB|SNB
Submitting a batchbuffer which simulates a gpu hang by doing
MI_BATCH_BUFFER_START into itself, to test hangcheck, started to hard hang the
whole box (IVB). Bisecting lead to this commit:
commit 664b422c2966cd39b8f67e8d53a566ea8c877cd6
Author: Vinit Azad
Date: Wed Aug 14 13:34:33 2013 -0700
drm/i915: Only unmask required PM interrupts
Experimenting with the mask register showed that unmasking EI UP will prevent
the hard hang in IVB and SNB.
HSW doesn't hang with EI UP masked.
Considering we are just disabling interrupts that aren't even delivered to
driver, this change is more likely to paper over some weirdness in gpu's
internal state machine. But until better explanation can be found, let's trade
little bit of power for stability on these architectures.
v2: - Unmask EI_EXPIRED directly in I915_WRITE (Vinit)
v3: - Only unmask on SNB and IVB
Cc: Vinit Azad
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_pm.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index cbab95dc..e6a124e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3447,14 +3447,24 @@ int intel_enable_rc6(const struct drm_device *dev)
static void gen6_enable_rps_interrupts(struct drm_device *dev) {
struct drm_i915_private *dev_priv = dev->dev_private;
+ u32 enabled_intrs;
spin_lock_irq(&dev_priv->irq_lock);
WARN_ON(dev_priv->rps.pm_iir);
snb_enable_pm_irq(dev_priv, GEN6_PM_RPS_EVENTS);
I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS);
spin_unlock_irq(&dev_priv->irq_lock);
+
/* only unmask PM interrupts we need. Mask all others. */
- I915_WRITE(GEN6_PMINTRMSK, ~GEN6_PM_RPS_EVENTS);
+ enabled_intrs = GEN6_PM_RPS_EVENTS;
+
+ /* IVB and SNB hard hangs on looping batchbuffer
+* if GEN6_PM_UP_EI_EXPIRED is masked.
+*/
+ if (INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev))
+ enabled_intrs |= GEN6_PM_RP_UP_EI_EXPIRED;
+
+ I915_WRITE(GEN6_PMINTRMSK, ~enabled_intrs);
}
static void gen6_enable_rps(struct drm_device *dev)
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] i915: Update VGA arbiter support for newer devices
On Fri, 2013-08-23 at 15:18 -0600, Alex Williamson wrote: > On Fri, 2013-08-23 at 21:21 +0300, Ville Syrjälä wrote: > > On Tue, Aug 20, 2013 at 10:46:45PM +0300, Ville Syrjälä wrote: > > > On Fri, Aug 16, 2013 at 12:22:14PM -0600, Alex Williamson wrote: > > > > On Fri, 2013-08-16 at 13:20 +0300, Ville Syrjälä wrote: > > > > > On Thu, Aug 15, 2013 at 04:54:15PM -0600, Alex Williamson wrote: > > > > > > On Fri, 2013-08-16 at 08:49 +1000, Dave Airlie wrote: > > > > > > > On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson > > > > > > > wrote: > > > > > > > > This is intended to add VGA arbiter support for Intel HD > > > > > > > > graphics on > > > > > > > > Core processors. The old GMCH registers no longer exist, so > > > > > > > > even > > > > > > > > though it appears that i915 participates in VGA arbitration, it > > > > > > > > doesn't > > > > > > > > work. On Intel HD graphics we already attempt to disable VGA > > > > > > > > regions > > > > > > > > of the device. This makes registering as a VGA client > > > > > > > > unnecessary since > > > > > > > > we don't intend to operate differently depending on how many > > > > > > > > VGA devices > > > > > > > > are present. We can disable VGA memory regions by clearing a > > > > > > > > memory > > > > > > > > enable bit in the VGA MSR. That only leaves VGA IO, which we > > > > > > > > update > > > > > > > > the VGA arbiter to know that we don't participate in VGA memory > > > > > > > > arbitration. We also add a hook on unload to re-enable memory > > > > > > > > and > > > > > > > > reinstate VGA memory arbitration. > > > > > > > > > > > > > > I would think there is still a VGA disable bit on the Intel device > > > > > > > somewhere, we'd just need > > > > > > > Intel to look in the docs and find it. A bit that can nuke both > > > > > > > i/o > > > > > > > and cmd regs. > > > > > > > > > > > > The only bit available is in the GGC and is a keyed/locked register > > > > > > that > > > > > > not only disables VGA memory and I/O, but also modifies the class > > > > > > code > > > > > > of the device. Early Core processors didn't lock this, but it's > > > > > > untouchable in newer ones AFAICT. Thanks, > > > > > > > > > > I've not found anything else in the docs. And also we _need_ VGA I/O > > > > > access to make i915_disable_vga() work. It's not 100% clear whether > > > > > we really need to poke at the sequencer register in modern hardware, > > > > > but the docs do still list it as a mandatory step. So even if we were > > > > > to have a global "disable VGA I/O and mem bit" we'd need to make sure > > > > > we already disabled VGA eg. after resume when the BIOS had a chance to > > > > > turn the VGA display back on. I think there were also some BIOSen that > > > > > turned VGA display back on when closing/opening the laptop lid. Not > > > > > sure what would even happen with those if totally disabled VGA I/O > > > > > access. I'm not sure they actually frob with the VGA regs though. > > > > > Could be they just turn on the VGA display bit in the VGA_CONTROL > > > > > register. > > > > > > > > Hmm, it appears the MSR write isn't fully disabling VGA memory space. > > > > When the VBIOS for the PEG graphics is run in the guest, I get some > > > > corruption of the IGD frame buffer. If I manually disable PCI memory in > > > > the command register, this doesn't happen. I also get some strange > > > > artifacts on the PEG display that don't happen when PCI memory is > > > > disabled. Should that MSR bit give us the whole a_-b_ range? > > > > > > Perhaps. It does that on some old graphics cards I've played with, but > > > frankly I have no idea what it does on our hardware. > > > > > > I'm trying to find out though. If and when I get an answer I'll let you > > > know. > > > > So the answer I basically got is that MSR is the only option here when > > the GMCH register can't be used. Supposedly it should work too, but > > I felt that I didn't get a 100% definite answer on that subject. > > I can imagine that the GMCH could be modified if we knew where the bit > was that's locking it. I can't find that in the spec though and I > assume that's intentional. > > > I tried it a bit on an IVB machine and PCI and PCIe Matrox G550 cards, > > and for me it does seem to work. In the G550 PCIe case there's an extra > > PCIe-PCI bridge on the card, and and in the G550 PCI case there's a > > PCI-PCI bridge on the card and a PCIe-PCI bridge on the motherboard. > > I don't have any native PCIe graphics cards on me to test the no > > extra bridges case. > > > > Based on a bit of manual register/memory banging it looks like the IGD > > will decode the access when MSR[1]=1, and won't when MSR[1]=0. Same was > > true for PCI_COMMAND[0] in case of VGA I/O. If those bits are disabled > > for IGD, the accesses get to the external card. If neither claims it, > > I just get 0xff back and writes are ignored. > > > > Curiously I didn't see any problems when I
Re: [Intel-gfx] [PATCH] i915: Update VGA arbiter support for newer devices
On Fri, 2013-08-23 at 21:21 +0300, Ville Syrjälä wrote: > On Tue, Aug 20, 2013 at 10:46:45PM +0300, Ville Syrjälä wrote: > > On Fri, Aug 16, 2013 at 12:22:14PM -0600, Alex Williamson wrote: > > > On Fri, 2013-08-16 at 13:20 +0300, Ville Syrjälä wrote: > > > > On Thu, Aug 15, 2013 at 04:54:15PM -0600, Alex Williamson wrote: > > > > > On Fri, 2013-08-16 at 08:49 +1000, Dave Airlie wrote: > > > > > > On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson > > > > > > wrote: > > > > > > > This is intended to add VGA arbiter support for Intel HD graphics > > > > > > > on > > > > > > > Core processors. The old GMCH registers no longer exist, so even > > > > > > > though it appears that i915 participates in VGA arbitration, it > > > > > > > doesn't > > > > > > > work. On Intel HD graphics we already attempt to disable VGA > > > > > > > regions > > > > > > > of the device. This makes registering as a VGA client > > > > > > > unnecessary since > > > > > > > we don't intend to operate differently depending on how many VGA > > > > > > > devices > > > > > > > are present. We can disable VGA memory regions by clearing a > > > > > > > memory > > > > > > > enable bit in the VGA MSR. That only leaves VGA IO, which we > > > > > > > update > > > > > > > the VGA arbiter to know that we don't participate in VGA memory > > > > > > > arbitration. We also add a hook on unload to re-enable memory and > > > > > > > reinstate VGA memory arbitration. > > > > > > > > > > > > I would think there is still a VGA disable bit on the Intel device > > > > > > somewhere, we'd just need > > > > > > Intel to look in the docs and find it. A bit that can nuke both i/o > > > > > > and cmd regs. > > > > > > > > > > The only bit available is in the GGC and is a keyed/locked register > > > > > that > > > > > not only disables VGA memory and I/O, but also modifies the class code > > > > > of the device. Early Core processors didn't lock this, but it's > > > > > untouchable in newer ones AFAICT. Thanks, > > > > > > > > I've not found anything else in the docs. And also we _need_ VGA I/O > > > > access to make i915_disable_vga() work. It's not 100% clear whether > > > > we really need to poke at the sequencer register in modern hardware, > > > > but the docs do still list it as a mandatory step. So even if we were > > > > to have a global "disable VGA I/O and mem bit" we'd need to make sure > > > > we already disabled VGA eg. after resume when the BIOS had a chance to > > > > turn the VGA display back on. I think there were also some BIOSen that > > > > turned VGA display back on when closing/opening the laptop lid. Not > > > > sure what would even happen with those if totally disabled VGA I/O > > > > access. I'm not sure they actually frob with the VGA regs though. > > > > Could be they just turn on the VGA display bit in the VGA_CONTROL > > > > register. > > > > > > Hmm, it appears the MSR write isn't fully disabling VGA memory space. > > > When the VBIOS for the PEG graphics is run in the guest, I get some > > > corruption of the IGD frame buffer. If I manually disable PCI memory in > > > the command register, this doesn't happen. I also get some strange > > > artifacts on the PEG display that don't happen when PCI memory is > > > disabled. Should that MSR bit give us the whole a_-b_ range? > > > > Perhaps. It does that on some old graphics cards I've played with, but > > frankly I have no idea what it does on our hardware. > > > > I'm trying to find out though. If and when I get an answer I'll let you > > know. > > So the answer I basically got is that MSR is the only option here when > the GMCH register can't be used. Supposedly it should work too, but > I felt that I didn't get a 100% definite answer on that subject. I can imagine that the GMCH could be modified if we knew where the bit was that's locking it. I can't find that in the spec though and I assume that's intentional. > I tried it a bit on an IVB machine and PCI and PCIe Matrox G550 cards, > and for me it does seem to work. In the G550 PCIe case there's an extra > PCIe-PCI bridge on the card, and and in the G550 PCI case there's a > PCI-PCI bridge on the card and a PCIe-PCI bridge on the motherboard. > I don't have any native PCIe graphics cards on me to test the no > extra bridges case. > > Based on a bit of manual register/memory banging it looks like the IGD > will decode the access when MSR[1]=1, and won't when MSR[1]=0. Same was > true for PCI_COMMAND[0] in case of VGA I/O. If those bits are disabled > for IGD, the accesses get to the external card. If neither claims it, > I just get 0xff back and writes are ignored. > > Curiously I didn't see any problems when I configured both graphics > devices and bridges to decode/forward VGA resources. The IGD was > always the one to answer and the data didn't seem corrupted. Not sure > why that is. Maybe I just got lucky or something. > > My tests weren't very thorough however, so I may have missed something
Re: [Intel-gfx] [PATCH v2] drm/i915: ivb: fix typo in dp voltage swing reg val
On Fri, Aug 23, 2013 at 11:50:23PM +0300, Imre Deak wrote: > Fix the typo introduced in > > commit 1a2eb4604b85c5efb343da8a4dcf41288fcfca85 > Author: Keith Packard > Date: Wed Nov 16 16:26:07 2011 -0800 > > drm/i915: Hook up Ivybridge eDP > > This fixes eDP link-training failures and cases where all voltage swing > /pre-emphasis levels were tried and failed during clock recovery and - > as a fallback - we go on to do channel equalization with the last voltage > swing/pre-emphasis level which will succeed. Both issues can lead to a > blank screen. > > v2: > - improve commit message > > CC: sta...@vger.kernel.org > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64880 > Tested-by: Jeremy Moles > Signed-off-by: Imre Deak Picked up for -fixes, thanks for the patch. Although I've frobbed the commit message a bit more and dropped the "typo" part. Since patches with "typo" headlines really don't belong into post -rc6 pull requests ;-) -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 v2] drm/i915: ivb: fix typo in dp voltage swing reg val
Fix the typo introduced in commit 1a2eb4604b85c5efb343da8a4dcf41288fcfca85 Author: Keith Packard Date: Wed Nov 16 16:26:07 2011 -0800 drm/i915: Hook up Ivybridge eDP This fixes eDP link-training failures and cases where all voltage swing /pre-emphasis levels were tried and failed during clock recovery and - as a fallback - we go on to do channel equalization with the last voltage swing/pre-emphasis level which will succeed. Both issues can lead to a blank screen. v2: - improve commit message CC: sta...@vger.kernel.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64880 Tested-by: Jeremy Moles Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/i915_reg.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 2b96d6b..019fd1f 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4525,7 +4525,7 @@ #define EDP_LINK_TRAIN_600MV_0DB_IVB (0x30 <<22) #define EDP_LINK_TRAIN_600MV_3_5DB_IVB (0x36 <<22) #define EDP_LINK_TRAIN_800MV_0DB_IVB (0x38 <<22) -#define EDP_LINK_TRAIN_800MV_3_5DB_IVB (0x33 <<22) +#define EDP_LINK_TRAIN_800MV_3_5DB_IVB (0x3e <<22) /* legacy values */ #define EDP_LINK_TRAIN_500MV_0DB_IVB (0x00 <<22) -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable
2013/8/23 Ville Syrjälä : > On Fri, Aug 23, 2013 at 04:41:57PM -0300, Paulo Zanoni wrote: >> 2013/8/23 Kristen Carlson Accardi : >> > On Fri, 23 Aug 2013 13:44:17 -0300 >> > Paulo Zanoni wrote: >> > >> >> 2013/8/23 Jani Nikula : >> >> >> >> /* Please insert explanation on why we need this and what changes if >> >> we do this. */ >> >> >> >> I applied your patches and booted them. I got into PC8, did the PC8 >> >> test suite and nothing changed. I really don't know what to expect >> >> from this series and/or how to check what's improving. Also, see >> >> below: >> >> >> > >> > So this is one of these things that will have no visible impact on >> > i915, but will impact other parts of the system. So I think the only >> > way to test it is by throwing it on the SIP board and checking the >> > power level of the components this impacts (Audio, thermal, KBC/EC, >> > LPT). And without the code which does the actual PCI D3 request from >> > i915, nothing will happen. >> >> I was told we could try to verify this by reading PCI config register >> 0xd4, but for me it's always 0x0, which means we're probably not >> really getting into D3. > > You have to write 0x3 into the PCI PM register to make it enter D3. > Exactly like you do when you suspend the whole machine. > > Not sure 0xd4 is the correct offset in this case, but you can look > it up from lspci output (remember +4), or in kernel code just use > pci_dev.pm_cap+4. Hmm, it's 0xd4 in my SNB at least. Maybe it's always > the same for the GPU. Check the description of 0xd4 on BSpec. I actually wrote the "get into D3" value to it (using setpci), and then when I got out of PC8 I saw tons and tons of error messages on dmesg due to the fact that we were failing to write registers. Which means it probably works, but we may have more work to do. > > -- > Ville Syrjälä > Intel OTC -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: ivb: fix typo in dp voltage swing reg val
On Fri, Aug 23, 2013 at 10:01:27PM +0200, Daniel Vetter wrote: > On Fri, Aug 23, 2013 at 04:01:17PM -0300, Paulo Zanoni wrote: > > 2013/8/23 Imre Deak : > > > Fix the typo introduced in > > > > > > commit 1a2eb4604b85c5efb343da8a4dcf41288fcfca85 > > > Author: Keith Packard > > > Date: Wed Nov 16 16:26:07 2011 -0800 > > > > > > drm/i915: Hook up Ivybridge eDP > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64880 > > > Tested-by: Jeremy Moles > > > Signed-off-by: Imre Deak > > > > Nice catch! > > > > Reviewed-by: Paulo Zanoni > > Queued for -next, thanks for the patch. Ok, dropped again since Paulo told me on irc that we fail link training without this and so this is a black screen fix and so should go to -fixes with cc: stable. Grumpy maintainer note: Stuff like this _really_ must be part of the commit message. A headline of "fix typo in ..." plus no mention of any further impact than the fixed type for a black screen bug in the commit message is seriously misleading. Also this should imo be caught in review (since Paulo obviously knew what's going on). Please resend. 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] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable
On Fri, Aug 23, 2013 at 04:41:57PM -0300, Paulo Zanoni wrote: > 2013/8/23 Kristen Carlson Accardi : > > On Fri, 23 Aug 2013 13:44:17 -0300 > > Paulo Zanoni wrote: > > > >> 2013/8/23 Jani Nikula : > >> > >> /* Please insert explanation on why we need this and what changes if > >> we do this. */ > >> > >> I applied your patches and booted them. I got into PC8, did the PC8 > >> test suite and nothing changed. I really don't know what to expect > >> from this series and/or how to check what's improving. Also, see > >> below: > >> > > > > So this is one of these things that will have no visible impact on > > i915, but will impact other parts of the system. So I think the only > > way to test it is by throwing it on the SIP board and checking the > > power level of the components this impacts (Audio, thermal, KBC/EC, > > LPT). And without the code which does the actual PCI D3 request from > > i915, nothing will happen. > > I was told we could try to verify this by reading PCI config register > 0xd4, but for me it's always 0x0, which means we're probably not > really getting into D3. You have to write 0x3 into the PCI PM register to make it enter D3. Exactly like you do when you suspend the whole machine. Not sure 0xd4 is the correct offset in this case, but you can look it up from lspci output (remember +4), or in kernel code just use pci_dev.pm_cap+4. Hmm, it's 0xd4 in my SNB at least. Maybe it's always the same for the GPU. -- 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: ivb: fix typo in dp voltage swing reg val
On Fri, Aug 23, 2013 at 04:01:17PM -0300, Paulo Zanoni wrote: > 2013/8/23 Imre Deak : > > Fix the typo introduced in > > > > commit 1a2eb4604b85c5efb343da8a4dcf41288fcfca85 > > Author: Keith Packard > > Date: Wed Nov 16 16:26:07 2011 -0800 > > > > drm/i915: Hook up Ivybridge eDP > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64880 > > Tested-by: Jeremy Moles > > Signed-off-by: Imre Deak > > Nice catch! > > Reviewed-by: Paulo Zanoni 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: write D_COMP using the mailbox
From: Paulo Zanoni
You can't write it using the MCHBAR mirror, the write will just get
dropped.
This should make us BSpec-compliant, but there's no real bug I could
reproduce that is fixed by this patch.
Signed-off-by: Paulo Zanoni
---
drivers/gpu/drm/i915/i915_reg.h | 4
drivers/gpu/drm/i915/intel_display.c | 10 --
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 53d0e70..bc0f4ab 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1397,6 +1397,8 @@
* device 0 function 0's pci config register 0x44 or 0x48 and matches it in
* every way. It is not accessible from the CP register read instructions.
*
+ * Starting form Haswell, you can't write registers using the MCHBAR mirror,
+ * just read.
*/
#define MCHBAR_MIRROR_BASE 0x1
@@ -4672,6 +4674,8 @@
#define GEN6_PCODE_READ_MIN_FREQ_TABLE 0x9
#define GEN6_PCODE_WRITE_RC6VIDS 0x4
#define GEN6_PCODE_READ_RC6VIDS 0x5
+#define GEN6_PCODE_READ_D_COMP 0x10
+#define GEN6_PCODE_WRITE_D_COMP 0x11
#define GEN6_ENCODE_RC6_VID(mv) (((mv) - 245) / 5)
#define GEN6_DECODE_RC6_VID(vids)(((vids) * 5) + 245)
#define GEN6_PCODE_DATA0x138128
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index c83152f..338b5d0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6000,7 +6000,10 @@ void hsw_disable_lcpll(struct drm_i915_private *dev_priv,
val = I915_READ(D_COMP);
val |= D_COMP_COMP_DISABLE;
- I915_WRITE(D_COMP, val);
+ mutex_lock(&dev_priv->rps.hw_lock);
+ if (sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_D_COMP, val))
+ DRM_ERROR("Failed to disable D_COMP\n");
+ mutex_unlock(&dev_priv->rps.hw_lock);
POSTING_READ(D_COMP);
ndelay(100);
@@ -6042,7 +6045,10 @@ void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
val = I915_READ(D_COMP);
val |= D_COMP_COMP_FORCE;
val &= ~D_COMP_COMP_DISABLE;
- I915_WRITE(D_COMP, val);
+ mutex_lock(&dev_priv->rps.hw_lock);
+ if (sandybridge_pcode_write(dev_priv, GEN6_PCODE_WRITE_D_COMP, val))
+ DRM_ERROR("Failed to enable D_COMP\n");
+ mutex_unlock(&dev_priv->rps.hw_lock);
POSTING_READ(D_COMP);
val = I915_READ(LCPLL_CTL);
--
1.8.1.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable
2013/8/23 Kristen Carlson Accardi :
> On Fri, 23 Aug 2013 13:44:17 -0300
> Paulo Zanoni wrote:
>
>> 2013/8/23 Jani Nikula :
>>
>> /* Please insert explanation on why we need this and what changes if
>> we do this. */
>>
>> I applied your patches and booted them. I got into PC8, did the PC8
>> test suite and nothing changed. I really don't know what to expect
>> from this series and/or how to check what's improving. Also, see
>> below:
>>
>
> So this is one of these things that will have no visible impact on
> i915, but will impact other parts of the system. So I think the only
> way to test it is by throwing it on the SIP board and checking the
> power level of the components this impacts (Audio, thermal, KBC/EC,
> LPT). And without the code which does the actual PCI D3 request from
> i915, nothing will happen.
I was told we could try to verify this by reading PCI config register
0xd4, but for me it's always 0x0, which means we're probably not
really getting into D3.
> Is it possible to get a patch which finds
> some very obvious place to put the controller into D3 so we can check
> to see if the opregion notifies are doing what they are supposed to?
http://cgit.freedesktop.org/~pzanoni/linux/?h=d3-wip
My check using the PCI config register 0xd4 suggests we're probably
not really enabling D3 :(
>
>>
>> > Signed-off-by: Jani Nikula
>> > ---
>> > 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 a6df68e..7ed2248 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -6093,6 +6093,8 @@ void hsw_enable_pc8_work(struct work_struct *__work)
>> > lpt_disable_clkout_dp(dev);
>> > hsw_pc8_disable_interrupts(dev);
>> > hsw_disable_lcpll(dev_priv, true, true);
>> > +
>> > + intel_opregion_notify_adapter(dev, PCI_D1);
>>
>> Why D1? Shouldn't this be D3? I think that's what people having been
>> asking us to implement.
>>
>> On the doc that explains "adapter power state notification", my
>> understanding is that it suggests that we should call this _before_ we
>> go into the lower states and the other chunk should be called _after_
>> we're at the higher power states. So perhaps we should call
>> intel_opregion_notify_adapter before hsw_disable_lcpll, and, on the
>> chunk below, after hsw_restore_lcpll? But this is not 100% clear, I
>> may be wrong.
>>
>> By the way, I modified your patch to implement the suggestions above,
>> and got the same results: no noticeable difference, everything still
>> works. No news is good news?
>>
>>
>> > }
>> >
>> > static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
>> > @@ -6126,6 +6128,8 @@ static void __hsw_disable_package_c8(struct
>> > drm_i915_private *dev_priv)
>> > if (!dev_priv->pc8.enabled)
>> > return;
>> >
>> > + intel_opregion_notify_adapter(dev, PCI_D0);
>> > +
>> > DRM_DEBUG_KMS("Disabling package C8+\n");
>> >
>> > hsw_restore_lcpll(dev_priv);
>> > --
>> > 1.7.9.5
>> >
>>
>>
>>
>
--
Paulo Zanoni
___
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, New testing round: - pc8+ support from Paulo - more vma patches from Ben. - Kconfig option to enable preliminary support by default (Josh Triplett) - Optimized cpu cache flush handling and support for write-through caching of display planes on Iris (Chris) - rc6 tuning from Stéphane Marchesin for more stability - VECS seqno wrap/semaphores fix (Ben) - a pile of smaller cleanups and improvements all over Note that I've dropped the execbuf vma conversion from Ben though since it felt a bit risky and I don't want to rock the 3.12 boat too much. I'll immediately reapply those again next for the new dinq. And a second heads-up: I expect that the next feature round won't hit 3.12, so from now on it's all for 3.13. Or at least very likely for 3.13. Happy 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] drm/i915: ivb: fix typo in dp voltage swing reg val
2013/8/23 Imre Deak : > Fix the typo introduced in > > commit 1a2eb4604b85c5efb343da8a4dcf41288fcfca85 > Author: Keith Packard > Date: Wed Nov 16 16:26:07 2011 -0800 > > drm/i915: Hook up Ivybridge eDP > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64880 > Tested-by: Jeremy Moles > Signed-off-by: Imre Deak Nice catch! Reviewed-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/i915_reg.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 2b96d6b..019fd1f 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4525,7 +4525,7 @@ > #define EDP_LINK_TRAIN_600MV_0DB_IVB (0x30 <<22) > #define EDP_LINK_TRAIN_600MV_3_5DB_IVB (0x36 <<22) > #define EDP_LINK_TRAIN_800MV_0DB_IVB (0x38 <<22) > -#define EDP_LINK_TRAIN_800MV_3_5DB_IVB (0x33 <<22) > +#define EDP_LINK_TRAIN_800MV_3_5DB_IVB (0x3e <<22) > > /* legacy values */ > #define EDP_LINK_TRAIN_500MV_0DB_IVB (0x00 <<22) > -- > 1.8.3.2 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: ivb: fix typo in dp voltage swing reg val
Fix the typo introduced in commit 1a2eb4604b85c5efb343da8a4dcf41288fcfca85 Author: Keith Packard Date: Wed Nov 16 16:26:07 2011 -0800 drm/i915: Hook up Ivybridge eDP Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=64880 Tested-by: Jeremy Moles Signed-off-by: Imre Deak --- drivers/gpu/drm/i915/i915_reg.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 2b96d6b..019fd1f 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4525,7 +4525,7 @@ #define EDP_LINK_TRAIN_600MV_0DB_IVB (0x30 <<22) #define EDP_LINK_TRAIN_600MV_3_5DB_IVB (0x36 <<22) #define EDP_LINK_TRAIN_800MV_0DB_IVB (0x38 <<22) -#define EDP_LINK_TRAIN_800MV_3_5DB_IVB (0x33 <<22) +#define EDP_LINK_TRAIN_800MV_3_5DB_IVB (0x3e <<22) /* legacy values */ #define EDP_LINK_TRAIN_500MV_0DB_IVB (0x00 <<22) -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] i915: Update VGA arbiter support for newer devices
On Tue, Aug 20, 2013 at 10:46:45PM +0300, Ville Syrjälä wrote: > On Fri, Aug 16, 2013 at 12:22:14PM -0600, Alex Williamson wrote: > > On Fri, 2013-08-16 at 13:20 +0300, Ville Syrjälä wrote: > > > On Thu, Aug 15, 2013 at 04:54:15PM -0600, Alex Williamson wrote: > > > > On Fri, 2013-08-16 at 08:49 +1000, Dave Airlie wrote: > > > > > On Fri, Aug 16, 2013 at 8:43 AM, Alex Williamson > > > > > wrote: > > > > > > This is intended to add VGA arbiter support for Intel HD graphics on > > > > > > Core processors. The old GMCH registers no longer exist, so even > > > > > > though it appears that i915 participates in VGA arbitration, it > > > > > > doesn't > > > > > > work. On Intel HD graphics we already attempt to disable VGA > > > > > > regions > > > > > > of the device. This makes registering as a VGA client unnecessary > > > > > > since > > > > > > we don't intend to operate differently depending on how many VGA > > > > > > devices > > > > > > are present. We can disable VGA memory regions by clearing a memory > > > > > > enable bit in the VGA MSR. That only leaves VGA IO, which we update > > > > > > the VGA arbiter to know that we don't participate in VGA memory > > > > > > arbitration. We also add a hook on unload to re-enable memory and > > > > > > reinstate VGA memory arbitration. > > > > > > > > > > I would think there is still a VGA disable bit on the Intel device > > > > > somewhere, we'd just need > > > > > Intel to look in the docs and find it. A bit that can nuke both i/o > > > > > and cmd regs. > > > > > > > > The only bit available is in the GGC and is a keyed/locked register that > > > > not only disables VGA memory and I/O, but also modifies the class code > > > > of the device. Early Core processors didn't lock this, but it's > > > > untouchable in newer ones AFAICT. Thanks, > > > > > > I've not found anything else in the docs. And also we _need_ VGA I/O > > > access to make i915_disable_vga() work. It's not 100% clear whether > > > we really need to poke at the sequencer register in modern hardware, > > > but the docs do still list it as a mandatory step. So even if we were > > > to have a global "disable VGA I/O and mem bit" we'd need to make sure > > > we already disabled VGA eg. after resume when the BIOS had a chance to > > > turn the VGA display back on. I think there were also some BIOSen that > > > turned VGA display back on when closing/opening the laptop lid. Not > > > sure what would even happen with those if totally disabled VGA I/O > > > access. I'm not sure they actually frob with the VGA regs though. > > > Could be they just turn on the VGA display bit in the VGA_CONTROL > > > register. > > > > Hmm, it appears the MSR write isn't fully disabling VGA memory space. > > When the VBIOS for the PEG graphics is run in the guest, I get some > > corruption of the IGD frame buffer. If I manually disable PCI memory in > > the command register, this doesn't happen. I also get some strange > > artifacts on the PEG display that don't happen when PCI memory is > > disabled. Should that MSR bit give us the whole a_-b_ range? > > Perhaps. It does that on some old graphics cards I've played with, but > frankly I have no idea what it does on our hardware. > > I'm trying to find out though. If and when I get an answer I'll let you > know. So the answer I basically got is that MSR is the only option here when the GMCH register can't be used. Supposedly it should work too, but I felt that I didn't get a 100% definite answer on that subject. I tried it a bit on an IVB machine and PCI and PCIe Matrox G550 cards, and for me it does seem to work. In the G550 PCIe case there's an extra PCIe-PCI bridge on the card, and and in the G550 PCI case there's a PCI-PCI bridge on the card and a PCIe-PCI bridge on the motherboard. I don't have any native PCIe graphics cards on me to test the no extra bridges case. Based on a bit of manual register/memory banging it looks like the IGD will decode the access when MSR[1]=1, and won't when MSR[1]=0. Same was true for PCI_COMMAND[0] in case of VGA I/O. If those bits are disabled for IGD, the accesses get to the external card. If neither claims it, I just get 0xff back and writes are ignored. Curiously I didn't see any problems when I configured both graphics devices and bridges to decode/forward VGA resources. The IGD was always the one to answer and the data didn't seem corrupted. Not sure why that is. Maybe I just got lucky or something. My tests weren't very thorough however, so I may have missed something. -- 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 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable
On Fri, 23 Aug 2013 13:44:17 -0300
Paulo Zanoni wrote:
> 2013/8/23 Jani Nikula :
>
> /* Please insert explanation on why we need this and what changes if
> we do this. */
>
> I applied your patches and booted them. I got into PC8, did the PC8
> test suite and nothing changed. I really don't know what to expect
> from this series and/or how to check what's improving. Also, see
> below:
>
So this is one of these things that will have no visible impact on
i915, but will impact other parts of the system. So I think the only
way to test it is by throwing it on the SIP board and checking the
power level of the components this impacts (Audio, thermal, KBC/EC,
LPT). And without the code which does the actual PCI D3 request from
i915, nothing will happen. Is it possible to get a patch which finds
some very obvious place to put the controller into D3 so we can check
to see if the opregion notifies are doing what they are supposed to?
>
> > Signed-off-by: Jani Nikula
> > ---
> > 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 a6df68e..7ed2248 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -6093,6 +6093,8 @@ void hsw_enable_pc8_work(struct work_struct *__work)
> > lpt_disable_clkout_dp(dev);
> > hsw_pc8_disable_interrupts(dev);
> > hsw_disable_lcpll(dev_priv, true, true);
> > +
> > + intel_opregion_notify_adapter(dev, PCI_D1);
>
> Why D1? Shouldn't this be D3? I think that's what people having been
> asking us to implement.
>
> On the doc that explains "adapter power state notification", my
> understanding is that it suggests that we should call this _before_ we
> go into the lower states and the other chunk should be called _after_
> we're at the higher power states. So perhaps we should call
> intel_opregion_notify_adapter before hsw_disable_lcpll, and, on the
> chunk below, after hsw_restore_lcpll? But this is not 100% clear, I
> may be wrong.
>
> By the way, I modified your patch to implement the suggestions above,
> and got the same results: no noticeable difference, everything still
> works. No news is good news?
>
>
> > }
> >
> > static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
> > @@ -6126,6 +6128,8 @@ static void __hsw_disable_package_c8(struct
> > drm_i915_private *dev_priv)
> > if (!dev_priv->pc8.enabled)
> > return;
> >
> > + intel_opregion_notify_adapter(dev, PCI_D0);
> > +
> > DRM_DEBUG_KMS("Disabling package C8+\n");
> >
> > hsw_restore_lcpll(dev_priv);
> > --
> > 1.7.9.5
> >
>
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] How will Gfx driver support runtime PM on Haswell?
On Fri, 23 Aug 2013 13:49:57 -0300 Paulo Zanoni wrote: > 2013/8/22 Jesse Barnes : > > On Wed, 21 Aug 2013 15:29:17 + > > "Lin, Mengdong" wrote: > > > >> Hi Ben, > >> > >> How will Gfx driver support runtime PM on Haswell? Will the Gfx driver > >> trigger "Adapter Power State" notification to BIOS when GPU switch state > >> between D0 and D3? > >> > >> We need to support RTD3 for HD-A legacy audio on Haswell Ultrabook. And > >> this depends on Gfx driver to implement two BIOS notifications. > >> > >> Here is the background of this dependency: > >> The target of HD-Audio RTD3 is that BIOS can power off the on-board 3rd > >> party audio codec to save power in S0. This would help to save ~60mW. > >> Please note that audio driver cannot power off the codec, because the > >> codec power is controlled by some GPIO which is board specific. > >> > >> The BIOS will power off the audio codec when three requirements are met: > >> > >> (1) System is running on battery > >> > >> (2) HD-Audio controller is in D3. Means no active audio application is > >> using the audio devices. > >> > >> (3) All displays are off. This means system is in "User-Absent Mode", > >> and a long latency for devices to resume back to D0 is allowed, such as > >> ~300ms to power on the audio codec. > >> > >> For the 3rd requirement, BIOS needs Gfx driver to tell whether the system > >> is in user-absent mode. It needs Gfx driver to triger two SCI SW > >> notifications: Display Power State Notification and Adapter Power State > >> Notification. > >> 'Display Power State Notification' was implemented by Jani. But we were > >> told that 'Adapter Power State' notification needs Gfx driver to support > >> runtime PM at first. > >> > >> So would you please share some information about runtime PM support in Gfx > >> driver? > > > > Paulo was going to test this on top of his PC8+ patchset. AFAFIK the > > only thing we need to do is put the device into D3 when we enter the > > PC8+ mode, and the BIOS will do the right thing (when we use the > > OpRegion stuff Jani did anyway). > > > > Paulo, are you still out celebrating the PC8 bits or have you had a > > chance to try this yet? > > From Mengdong's email, it seems Jani's patch series implements exactly > what was requested, except for the fact that our patch says "D1" > instead of "D3" to the BIOS. Is there something else we need besides > this? I don't think so... well except for testing. :) If you have something that seems to work it's probably worthwhile to get Kristen to try it out on her instrumented setup. -- Jesse Barnes, 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] How will Gfx driver support runtime PM on Haswell?
2013/8/22 Jesse Barnes : > On Wed, 21 Aug 2013 15:29:17 + > "Lin, Mengdong" wrote: > >> Hi Ben, >> >> How will Gfx driver support runtime PM on Haswell? Will the Gfx driver >> trigger "Adapter Power State" notification to BIOS when GPU switch state >> between D0 and D3? >> >> We need to support RTD3 for HD-A legacy audio on Haswell Ultrabook. And this >> depends on Gfx driver to implement two BIOS notifications. >> >> Here is the background of this dependency: >> The target of HD-Audio RTD3 is that BIOS can power off the on-board 3rd >> party audio codec to save power in S0. This would help to save ~60mW. >> Please note that audio driver cannot power off the codec, because the codec >> power is controlled by some GPIO which is board specific. >> >> The BIOS will power off the audio codec when three requirements are met: >> >> (1) System is running on battery >> >> (2) HD-Audio controller is in D3. Means no active audio application is >> using the audio devices. >> >> (3) All displays are off. This means system is in "User-Absent Mode", >> and a long latency for devices to resume back to D0 is allowed, such as >> ~300ms to power on the audio codec. >> >> For the 3rd requirement, BIOS needs Gfx driver to tell whether the system is >> in user-absent mode. It needs Gfx driver to triger two SCI SW notifications: >> Display Power State Notification and Adapter Power State Notification. >> 'Display Power State Notification' was implemented by Jani. But we were told >> that 'Adapter Power State' notification needs Gfx driver to support runtime >> PM at first. >> >> So would you please share some information about runtime PM support in Gfx >> driver? > > Paulo was going to test this on top of his PC8+ patchset. AFAFIK the > only thing we need to do is put the device into D3 when we enter the > PC8+ mode, and the BIOS will do the right thing (when we use the > OpRegion stuff Jani did anyway). > > Paulo, are you still out celebrating the PC8 bits or have you had a > chance to try this yet? >From Mengdong's email, it seems Jani's patch series implements exactly what was requested, except for the fact that our patch says "D1" instead of "D3" to the BIOS. Is there something else we need besides this? > > -- > Jesse Barnes, Intel Open Source Technology Center -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable
2013/8/23 Jani Nikula :
/* Please insert explanation on why we need this and what changes if
we do this. */
I applied your patches and booted them. I got into PC8, did the PC8
test suite and nothing changed. I really don't know what to expect
from this series and/or how to check what's improving. Also, see
below:
> Signed-off-by: Jani Nikula
> ---
> 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 a6df68e..7ed2248 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6093,6 +6093,8 @@ void hsw_enable_pc8_work(struct work_struct *__work)
> lpt_disable_clkout_dp(dev);
> hsw_pc8_disable_interrupts(dev);
> hsw_disable_lcpll(dev_priv, true, true);
> +
> + intel_opregion_notify_adapter(dev, PCI_D1);
Why D1? Shouldn't this be D3? I think that's what people having been
asking us to implement.
On the doc that explains "adapter power state notification", my
understanding is that it suggests that we should call this _before_ we
go into the lower states and the other chunk should be called _after_
we're at the higher power states. So perhaps we should call
intel_opregion_notify_adapter before hsw_disable_lcpll, and, on the
chunk below, after hsw_restore_lcpll? But this is not 100% clear, I
may be wrong.
By the way, I modified your patch to implement the suggestions above,
and got the same results: no noticeable difference, everything still
works. No news is good news?
> }
>
> static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
> @@ -6126,6 +6128,8 @@ static void __hsw_disable_package_c8(struct
> drm_i915_private *dev_priv)
> if (!dev_priv->pc8.enabled)
> return;
>
> + intel_opregion_notify_adapter(dev, PCI_D0);
> +
> DRM_DEBUG_KMS("Disabling package C8+\n");
>
> hsw_restore_lcpll(dev_priv);
> --
> 1.7.9.5
>
--
Paulo Zanoni
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 12/15] drm/i915: Band Gap WA
On Fri, Aug 16, 2013 at 03:36:00PM +0300, Jani Nikula wrote:
> From: Shobhit Kumar
>
> Signed-off-by: Shobhit Kumar
> Signed-off-by: ymohanma
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/i915/intel_dsi.c | 48
> ++
> 1 file changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> b/drivers/gpu/drm/i915/intel_dsi.c
> index 352ff46..676b310 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -37,6 +37,51 @@
> static const struct intel_dsi_device intel_dsi_devices[] = {
> };
>
> +
> +static void vlv_cck_modify(struct drm_i915_private *dev_priv, u32 reg, u32
> val,
> +u32 mask)
> +{
> + u32 tmp = vlv_cck_read(dev_priv, reg);
> + tmp &= ~mask;
> + tmp |= val;
> + vlv_cck_write(dev_priv, reg, tmp);
> +}
> +
> +static void band_gap_wa(struct drm_i915_private *dev_priv)
> +{
> + mutex_lock(&dev_priv->dpio_lock);
> +
> + /* Enable bandgap fix in GOP driver */
> + vlv_cck_modify(dev_priv, 0x6D, 0x0001, 0x0003);
> + msleep(20);
> + vlv_cck_modify(dev_priv, 0x6E, 0x0001, 0x0003);
> + msleep(20);
> + vlv_cck_modify(dev_priv, 0x6F, 0x0001, 0x0003);
> + msleep(20);
> + vlv_cck_modify(dev_priv, 0x00, 0x8000, 0x8000);
> + msleep(20);
> + vlv_cck_modify(dev_priv, 0x00, 0x, 0x8000);
> + msleep(20);
> +
> + /* Turn Display Trunk on */
This single comment here looks a bit inconsistent. Why have it for this
one case and not the others. Maybe we should just give the bits proper
names so we don't need such silly comments.
> + vlv_cck_modify(dev_priv, 0x6B, 0x0002, 0x0003);
> + msleep(20);
> +
> + vlv_cck_modify(dev_priv, 0x6C, 0x0002, 0x0003);
> + msleep(20);
> +
> + vlv_cck_modify(dev_priv, 0x6D, 0x0002, 0x0003);
> + msleep(20);
> + vlv_cck_modify(dev_priv, 0x6E, 0x0002, 0x0003);
> + msleep(20);
> + vlv_cck_modify(dev_priv, 0x6F, 0x0002, 0x0003);
I wonder if we really need to force all the trunks on, or would it be
enough to clear the force off bits and rely on the signals from Punit
to do the rest. I suppose we should at least clear the DSI trunk force
on bits when disabling DSI output.
Can't say anything intelligent about the workaround in general as I've
not seen it mentioned anywhere. Based on the spec this appears to just
force the DSI trunk clocks off, then toggle some DSI releated reset,
and then force the clocks back on (also the dp and dpref trunks, even
though it didn't actually force them off in the first place, which seems
a bit weird).
Also wondering about all these sleeps. Do we really need them at every
step, or could we just disable all the trunk clocks -> sleep -> reset on ->
sleep -> reset off -> sleep -> turn the trunks back on -> sleep some
more. That's almost 300ms were sleeping here, and that's not even including
the sideband overhead.
> +
> + mutex_unlock(&dev_priv->dpio_lock);
> +
> + /* Need huge delay, otherwise clock is not stable */
> + msleep(100);
> +}
> +
> static struct intel_dsi *intel_attached_dsi(struct drm_connector *connector)
> {
> return container_of(intel_attached_encoder(connector),
> @@ -291,6 +336,9 @@ static void intel_dsi_mode_set(struct intel_encoder
> *intel_encoder)
> /* Update the DSI PLL */
> vlv_enable_dsi_pll(intel_encoder);
>
> + /* XXX: Location of the call */
> + band_gap_wa(dev_priv);
> +
> /* escape clock divider, 20MHz, shared for A and C. device ready must be
>* off when doing this! txclkesc? */
> tmp = I915_READ(MIPI_CTRL(0));
> --
> 1.7.9.5
>
> ___
> 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
[Intel-gfx] [PATCH 2/2] drm/i915: ban badly behaving contexts
Now when we have mechanism in place to track which context
was guilty of hanging the gpu, it is possible to punish
for bad behaviour.
If context has recently submitted a faulty batchbuffers guilty of
gpu hang and submits another batch which hangs gpu in a 16 seconds
time window, ban it permanently.
Ban means that batchbuffers from this fd/context will not
be accepted anymore for execution.
v2: Store guilty ban status bool in gpu_error instead of pointers
that might become danling before hang is declared.
v3: Use return value for banned status instead of stashing state
into gpu_error (Chris Wilson)
v4: - rebase on top of fixed hang stats api
- add define for ban period
- rename commit and improve commit msg
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_drv.c|6 +++--
drivers/gpu/drm/i915/i915_drv.h| 10 +++-
drivers/gpu/drm/i915/i915_gem.c| 35 +---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 12 ++
4 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e19dec5..8934365 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -804,6 +804,7 @@ int i915_reset(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = dev->dev_private;
bool simulated;
+ bool ctx_banned;
int ret;
if (!i915_try_reset)
@@ -811,11 +812,12 @@ int i915_reset(struct drm_device *dev)
mutex_lock(&dev->struct_mutex);
- i915_gem_reset(dev);
+ ctx_banned = i915_gem_reset(dev);
simulated = dev_priv->gpu_error.stop_rings != 0;
- if (!simulated && get_seconds() - dev_priv->gpu_error.last_reset <
+ if (!(simulated || ctx_banned) &&
+ get_seconds() - dev_priv->gpu_error.last_reset <
DRM_I915_WEDGE_PERIOD) {
DRM_ERROR("GPU hanging too fast, declaring wedged!\n");
ret = -ENODEV;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9c0ca78..9d99937 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -575,6 +575,12 @@ struct i915_ctx_hang_stats {
/* This context had batch active when hang was declared */
unsigned batch_active;
+
+ /* Time when this context was last blamed for a GPU reset */
+ unsigned long batch_active_reset_ts;
+
+ /* This context is banned to submit more work */
+ bool banned;
};
/* This must match up with the value previously used for execbuf2.rsvd1. */
@@ -978,6 +984,8 @@ struct i915_gpu_error {
#define DRM_I915_HANGCHECK_JIFFIES msecs_to_jiffies(DRM_I915_HANGCHECK_PERIOD)
/* Hangcheck needs >2 periods to declare a hang */
#define DRM_I915_WEDGE_PERIOD DIV_ROUND_UP(5*DRM_I915_HANGCHECK_PERIOD, 1000)
+ /* Hang gpu twice in this window and your context gets banned */
+#define DRM_I915_CONTEXT_BAN_PERIOD (2 * DRM_I915_WEDGE_PERIOD)
struct timer_list hangcheck_timer;
@@ -1922,7 +1930,7 @@ static inline bool i915_terminally_wedged(struct
i915_gpu_error *error)
return atomic_read(&error->reset_counter) == I915_WEDGED;
}
-void i915_gem_reset(struct drm_device *dev);
+bool i915_gem_reset(struct drm_device *dev);
bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, bool force);
int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj);
int __must_check i915_gem_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 23c4256..0592966 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2201,16 +2201,16 @@ static bool i915_request_guilty(struct
drm_i915_gem_request *request,
return false;
}
-static void i915_set_reset_status(struct intel_ring_buffer *ring,
+static bool i915_set_reset_status(struct intel_ring_buffer *ring,
struct drm_i915_gem_request *request,
u32 acthd)
{
struct i915_ctx_hang_stats *hs = NULL;
- bool inside, guilty;
+ bool inside, guilty, banned;
unsigned long offset = 0;
/* Innocent until proven guilty */
- guilty = false;
+ guilty = banned = false;
if (request->batch_obj)
offset = i915_gem_obj_offset(request->batch_obj,
@@ -2237,11 +2237,21 @@ static void i915_set_reset_status(struct
intel_ring_buffer *ring,
hs = &request->file_priv->hang_stats;
if (hs) {
- if (guilty)
+ if (guilty) {
+ if (!hs->banned &&
+ get_seconds() - hs->batch_active_reset_ts <
+ DRM_I915_CONTEXT_BAN_PERIOD) {
+ hs->banned = banned = true;
+ DRM_ERROR("context hanging too
[Intel-gfx] [PATCH 1/2] drm/i915: increase GPU wedging timeout
Currently our wedge timeout is 5 seconds. Hangcheck
needs atleast three runs to declare a hang with 1500ms
timer tick period.
To make sure that gpu can be wedged in the first place,
define wedge timeout as multiple of hangcheck timer periods to ensure
that it is always greater than hang detection time.
This commit increases wedging period from 5 seconds to 8 seconds.
v2: better name for macro (Chris Wilson)
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_drv.c |3 ++-
drivers/gpu/drm/i915/i915_drv.h |3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index beb2956..e19dec5 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -815,7 +815,8 @@ int i915_reset(struct drm_device *dev)
simulated = dev_priv->gpu_error.stop_rings != 0;
- if (!simulated && get_seconds() - dev_priv->gpu_error.last_reset < 5) {
+ if (!simulated && get_seconds() - dev_priv->gpu_error.last_reset <
+ DRM_I915_WEDGE_PERIOD) {
DRM_ERROR("GPU hanging too fast, declaring wedged!\n");
ret = -ENODEV;
} else {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5f8a638..9c0ca78 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -976,6 +976,9 @@ struct i915_gpu_error {
/* For hangcheck timer */
#define DRM_I915_HANGCHECK_PERIOD 1500 /* in ms */
#define DRM_I915_HANGCHECK_JIFFIES msecs_to_jiffies(DRM_I915_HANGCHECK_PERIOD)
+ /* Hangcheck needs >2 periods to declare a hang */
+#define DRM_I915_WEDGE_PERIOD DIV_ROUND_UP(5*DRM_I915_HANGCHECK_PERIOD, 1000)
+
struct timer_list hangcheck_timer;
/* For reset and error_state handling. */
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: sanitize forcewake registers on reset
In reset we try to restore the forcewake state to
pre reset state, using forcewake_count. The reset
doesn't seem to clear the forcewake bits so we
get warn on forcewake ack register not clearing.
Use same mechanism as intel_uncore_sanitize() does
when loading driver to reset the forcewake bits, right
after the chip has been reset.
Reviewed-by: Chris Wilson
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_uncore.c |9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c
b/drivers/gpu/drm/i915/intel_uncore.c
index 8f5bc86..8649f1c 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -261,7 +261,7 @@ void intel_uncore_init(struct drm_device *dev)
}
}
-void intel_uncore_sanitize(struct drm_device *dev)
+static void intel_uncore_forcewake_reset(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -272,6 +272,11 @@ void intel_uncore_sanitize(struct drm_device *dev)
if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
__gen6_gt_force_wake_mt_reset(dev_priv);
}
+}
+
+void intel_uncore_sanitize(struct drm_device *dev)
+{
+ intel_uncore_forcewake_reset(dev);
/* BIOS often leaves RC6 enabled, but disable it for hw init */
intel_disable_gt_powersave(dev);
@@ -549,6 +554,8 @@ static int gen6_do_reset(struct drm_device *dev)
/* Spin waiting for the device to ack the reset request */
ret = wait_for((__raw_i915_read32(dev_priv, GEN6_GDRST) &
GEN6_GRDOM_FULL) == 0, 500);
+ intel_uncore_forcewake_reset(dev);
+
/* If reset with a user forcewake, try to restore, otherwise turn it
off */
if (dev_priv->uncore.forcewake_count)
dev_priv->uncore.funcs.force_wake_get(dev_priv);
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Print seqnos as unsigned in debugfs
On Fri, Aug 23, 2013 at 12:12:14PM +0100, Damien Lespiau wrote: > On Thu, Aug 22, 2013 at 07:21:30PM +0300, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > I don't like seeing signed seqnos. Make them unsigned. > > > > Signed-off-by: Ville Syrjälä > > Reviewed-by: Damien Lespiau 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] Fix for the i915_vma_unbind() fix.
On Fri, Aug 23, 2013 at 10:07:16AM +0100, Chris Wilson wrote:
> We need to move the bogus warn out of the way and only do the
> vma->vma_link decoupling before destroying the node.
>
> Fixes gem_evict_everything, gem_evict_alignment
>
> Squash in with Daniel's fixup to the fix -- I expect that he has already
> done so...
>
> Signed-off-by: Chris Wilson
Hm, I don't see the functional difference to the resend patch I've posted
last night... still mail woes?
And QA reported that this approach is still not good enough to make :(
Since I don't want to jeopardize other feature work I'll take out the vma
execbuf patch now so that I can cut a new -testing. We can resume the
head-banging next week ;-)
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 5788e9d..744f9a6 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2725,6 +2725,9 @@ int i915_vma_unbind(struct i915_vma *vma)
> if (list_empty(&vma->vma_link))
> return 0;
>
> + /* NB: Until we have real VMAs there will only ever be one */
> + WARN_ON(!list_is_singular(&obj->vma_list));
> +
> if (!drm_mm_node_allocated(&vma->node))
> goto destroy;
>
> @@ -2771,8 +2774,7 @@ destroy:
>
> /* Since the unbound list is global, only move to that list if
>* no more VMAs exist.
> - * NB: Until we have real VMAs there will only ever be one */
> - WARN_ON(!list_empty(&obj->vma_list));
> + */
> if (list_empty(&obj->vma_list))
> list_move_tail(&obj->global_list, &dev_priv->mm.unbound_list);
>
> @@ -4279,12 +4281,12 @@ i915_gem_obj_lookup_or_create_vma(struct
> drm_i915_gem_object *obj,
> void i915_gem_vma_destroy(struct i915_vma *vma)
> {
> WARN_ON(vma->node.allocated);
> - list_del(&vma->vma_link);
>
> /* Keep the vma as a placeholder in the execbuffer reservation lists */
> if (!list_empty(&vma->exec_list))
> return;
>
> + list_del(&vma->vma_link);
> kfree(vma);
> }
>
> --
> 1.8.4.rc3
>
> ___
> 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
[Intel-gfx] [PATCH] drm/i915: Fix __wait_seqno to use true infinite timeouts
When we switched to always using a timeout in conjunction with
wait_seqno, we lost the ability to detect missed interrupts. Since, we
have had issues with interrupts on a number of generations, and they are
required to be delivered in a timely fashion for a smooth UX, it is
important that we do log errors found in the wild and prevent the
display stalling for upwards of 1s every time the seqno interrupt is
missed.
Rather than continue to fix up the timeouts to work around the interface
impedence in wait_event_*(), open code the combination of
wait_event[_interruptible][_timeout], and use the exposed timer to
poll for seqno should we detect a lost interrupt.
v2: In order to satisfy the debug requirement of logging missed
interrupts with the real world requirments of making machines work even
if interrupts are hosed, we revert to polling after detecting a missed
interrupt.
v3: Throw in a debugfs interface to simulate broken hw not reporting
interrupts.
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_debugfs.c | 68
drivers/gpu/drm/i915/i915_drv.h | 5 ++
drivers/gpu/drm/i915/i915_gem.c | 114 --
drivers/gpu/drm/i915/i915_gpu_error.c | 1 +
drivers/gpu/drm/i915/i915_irq.c | 11 ++--
5 files changed, 148 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
b/drivers/gpu/drm/i915/i915_debugfs.c
index 8356254..320fb03 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1870,6 +1870,72 @@ DEFINE_SIMPLE_ATTRIBUTE(i915_ring_stop_fops,
i915_ring_stop_get, i915_ring_stop_set,
"0x%08llx\n");
+static int
+i915_ring_missed_irq_get(void *data, u64 *val)
+{
+ struct drm_device *dev = data;
+ drm_i915_private_t *dev_priv = dev->dev_private;
+
+ *val = dev_priv->gpu_error.missed_irq_rings;
+ return 0;
+}
+
+static int
+i915_ring_missed_irq_set(void *data, u64 val)
+{
+ struct drm_device *dev = data;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ int ret;
+
+ /* Lock against concurrent debugfs callers */
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
+ dev_priv->gpu_error.missed_irq_rings = val;
+ mutex_unlock(&dev->struct_mutex);
+
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_ring_missed_irq_fops,
+ i915_ring_missed_irq_get, i915_ring_missed_irq_set,
+ "0x%08llx\n");
+
+static int
+i915_ring_test_irq_get(void *data, u64 *val)
+{
+ struct drm_device *dev = data;
+ drm_i915_private_t *dev_priv = dev->dev_private;
+
+ *val = dev_priv->gpu_error.test_irq_rings;
+
+ return 0;
+}
+
+static int
+i915_ring_test_irq_set(void *data, u64 val)
+{
+ struct drm_device *dev = data;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ int ret;
+
+ DRM_DEBUG_DRIVER("Masking interrupts on rings 0x%08llx\n", val);
+
+ /* Lock against concurrent debugfs callers */
+ ret = mutex_lock_interruptible(&dev->struct_mutex);
+ if (ret)
+ return ret;
+
+ dev_priv->gpu_error.test_irq_rings = val;
+ mutex_unlock(&dev->struct_mutex);
+
+ return 0;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_ring_test_irq_fops,
+ i915_ring_test_irq_get, i915_ring_test_irq_set,
+ "0x%08llx\n");
+
#define DROP_UNBOUND 0x1
#define DROP_BOUND 0x2
#define DROP_RETIRE 0x4
@@ -2263,6 +2329,8 @@ static struct i915_debugfs_files {
{"i915_min_freq", &i915_min_freq_fops},
{"i915_cache_sharing", &i915_cache_sharing_fops},
{"i915_ring_stop", &i915_ring_stop_fops},
+ {"i915_ring_missed_irq", &i915_ring_missed_irq_fops},
+ {"i915_ring_test_irq", &i915_ring_test_irq_fops},
{"i915_gem_drop_caches", &i915_drop_caches_fops},
{"i915_error_state", &i915_error_state_fops},
{"i915_next_seqno", &i915_next_seqno_fops},
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 321159f..45c7790 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -994,6 +994,8 @@ struct i915_gpu_error {
unsigned long last_reset;
+ unsigned long missed_irq_rings;
+
/**
* State variable and reset counter controlling the reset flow
*
@@ -1032,6 +1034,9 @@ struct i915_gpu_error {
/* For gpu hang simulation. */
unsigned int stop_rings;
+
+ /* For missed irq/seqno simulation. */
+ unsigned int test_irq_rings;
};
enum modeset_restore {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 4f1ca12..a8034c0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1067,6 +1067,17 @@ i915_gem_check_olr(struct intel_ring_buffer *ring, u32
seqno
Re: [Intel-gfx] [PATCH] drm/i915: Print seqnos as unsigned in debugfs
On Thu, Aug 22, 2013 at 07:21:30PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> I don't like seeing signed seqnos. Make them unsigned.
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Damien Lespiau
--
Damien
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 236d97e..49ca4d8 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -100,7 +100,7 @@ static void
> describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
> {
> struct i915_vma *vma;
> - seq_printf(m, "%pK: %s%s%s %8zdKiB %02x %02x %d %d %d%s%s%s",
> + seq_printf(m, "%pK: %s%s%s %8zdKiB %02x %02x %u %u %u%s%s%s",
> &obj->base,
> get_pin_flag(obj),
> get_tiling_flag(obj),
> --
> 1.8.1.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
[Intel-gfx] [PATCH 5/6] DRAFT: drm/i915: do display power state notification on crtc enable/disable
The spec says to notify prior to power down and after power up. It is
unclear whether it makes a difference.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_display.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index bcb62fe..a6df68e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3404,8 +3404,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
intel_update_fbc(dev);
mutex_unlock(&dev->struct_mutex);
- for_each_encoder_on_crtc(dev, crtc, encoder)
+ for_each_encoder_on_crtc(dev, crtc, encoder) {
encoder->enable(encoder);
+ intel_opregion_notify_encoder(encoder, true);
+ }
/*
* There seems to be a race in PCH platform hw (at least on some
@@ -3519,8 +3521,10 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
if (!intel_crtc->active)
return;
- for_each_encoder_on_crtc(dev, crtc, encoder)
+ for_each_encoder_on_crtc(dev, crtc, encoder) {
+ intel_opregion_notify_encoder(encoder, false);
encoder->disable(encoder);
+ }
intel_crtc_wait_for_pending_flips(crtc);
drm_vblank_off(dev, pipe);
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/6] DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable
Signed-off-by: Jani Nikula
---
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 a6df68e..7ed2248 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6093,6 +6093,8 @@ void hsw_enable_pc8_work(struct work_struct *__work)
lpt_disable_clkout_dp(dev);
hsw_pc8_disable_interrupts(dev);
hsw_disable_lcpll(dev_priv, true, true);
+
+ intel_opregion_notify_adapter(dev, PCI_D1);
}
static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
@@ -6126,6 +6128,8 @@ static void __hsw_disable_package_c8(struct
drm_i915_private *dev_priv)
if (!dev_priv->pc8.enabled)
return;
+ intel_opregion_notify_adapter(dev, PCI_D0);
+
DRM_DEBUG_KMS("Disabling package C8+\n");
hsw_restore_lcpll(dev_priv);
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/6] drm/i915: add opregion function to notify bios of adapter power state
Notifying the bios lets it enter power saving states.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_drv.h |3 +++
drivers/gpu/drm/i915/intel_opregion.c | 27 +++
2 files changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1703029..e17a9a0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2179,12 +2179,15 @@ extern void intel_opregion_fini(struct drm_device *dev);
extern void intel_opregion_asle_intr(struct drm_device *dev);
extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
bool enable);
+extern int intel_opregion_notify_adapter(struct drm_device *dev,
+pci_power_t state);
#else
static inline void intel_opregion_init(struct drm_device *dev) { return; }
static inline void intel_opregion_fini(struct drm_device *dev) { return; }
static inline void intel_opregion_asle_intr(struct drm_device *dev) { return; }
static inline int
intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, bool enable)
+intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
{
return 0;
}
diff --git a/drivers/gpu/drm/i915/intel_opregion.c
b/drivers/gpu/drm/i915/intel_opregion.c
index 72a4af6..e47aefc 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -325,6 +325,33 @@ int intel_opregion_notify_encoder(struct intel_encoder
*intel_encoder,
return swsci(dev, SWSCI_SBCB_DISPLAY_POWER_STATE, parm, NULL);
}
+static const struct {
+ pci_power_t pci_power_state;
+ u32 parm;
+} power_state_map[] = {
+ { PCI_D0, 0x00 },
+ { PCI_D1, 0x01 },
+ { PCI_D2, 0x02 },
+ { PCI_D3hot,0x04 },
+ { PCI_D3cold, 0x04 },
+};
+
+int intel_opregion_notify_adapter(struct drm_device *dev, pci_power_t state)
+{
+ int i;
+
+ if (!HAS_DDI(dev))
+ return 0;
+
+ for (i = 0; i < ARRAY_SIZE(power_state_map); i++) {
+ if (state == power_state_map[i].pci_power_state)
+ return swsci(dev, SWSCI_SBCB_ADAPTER_POWER_STATE,
+power_state_map[i].parm, NULL);
+ }
+
+ return -EINVAL;
+}
+
static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
{
struct drm_i915_private *dev_priv = dev->dev_private;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/6] drm/i915: add opregion function to notify bios of encoder enable/disable
The bios interface seems messy, and it's hard to tell what the bios
really wants. At first, only add support for DDI based machines (hsw+),
and see how it turns out.
The spec says to notify prior to power down and after power up. It is
unclear whether it makes a difference.
v2:
- squash notification function and callers patches together (Daniel)
- move callers to haswell_crtc_{enable,disable} (Daniel)
- rename notification function (Chris)
v3:
- separate notification function and callers again, as it's not clear
whether the display power state notification is the right thing to do
after all
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_drv.h |8 +
drivers/gpu/drm/i915/intel_opregion.c | 52 +
2 files changed, 60 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index adc2f46..1703029 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2171,15 +2171,23 @@ static inline bool intel_gmbus_is_forced_bit(struct
i2c_adapter *adapter)
extern void intel_i2c_reset(struct drm_device *dev);
/* intel_opregion.c */
+struct intel_encoder;
extern int intel_opregion_setup(struct drm_device *dev);
#ifdef CONFIG_ACPI
extern void intel_opregion_init(struct drm_device *dev);
extern void intel_opregion_fini(struct drm_device *dev);
extern void intel_opregion_asle_intr(struct drm_device *dev);
+extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
+bool enable);
#else
static inline void intel_opregion_init(struct drm_device *dev) { return; }
static inline void intel_opregion_fini(struct drm_device *dev) { return; }
static inline void intel_opregion_asle_intr(struct drm_device *dev) { return; }
+static inline int
+intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, bool enable)
+{
+ return 0;
+}
#endif
/* intel_acpi.c */
diff --git a/drivers/gpu/drm/i915/intel_opregion.c
b/drivers/gpu/drm/i915/intel_opregion.c
index a3558ae..72a4af6 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -273,6 +273,58 @@ static int swsci(struct drm_device *dev, u32 function, u32
parm, u32 *parm_out)
#undef C
}
+#define DISPLAY_TYPE_CRT 0
+#define DISPLAY_TYPE_TV1
+#define DISPLAY_TYPE_EXTERNAL_FLAT_PANEL 2
+#define DISPLAY_TYPE_INTERNAL_FLAT_PANEL 3
+
+int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
+ bool enable)
+{
+ struct drm_device *dev = intel_encoder->base.dev;
+ u32 parm = 0;
+ u32 type = 0;
+ u32 port;
+
+ /* don't care about old stuff for now */
+ if (!HAS_DDI(dev))
+ return 0;
+
+ port = intel_ddi_get_encoder_port(intel_encoder);
+ if (port == PORT_E) {
+ port = 0;
+ } else {
+ parm |= 1 << port;
+ port++;
+ }
+
+ if (!enable)
+ parm |= 4 << 8;
+
+ switch (intel_encoder->type) {
+ case INTEL_OUTPUT_ANALOG:
+ type = DISPLAY_TYPE_CRT;
+ break;
+ case INTEL_OUTPUT_UNKNOWN:
+ case INTEL_OUTPUT_DISPLAYPORT:
+ case INTEL_OUTPUT_HDMI:
+ type = DISPLAY_TYPE_EXTERNAL_FLAT_PANEL;
+ break;
+ case INTEL_OUTPUT_LVDS:
+ case INTEL_OUTPUT_EDP:
+ type = DISPLAY_TYPE_INTERNAL_FLAT_PANEL;
+ break;
+ default:
+ DRM_DEBUG_DRIVER("unsupported intel_encoder type %d\n",
+intel_encoder->type);
+ return -EINVAL;
+ }
+
+ parm |= type << (16 + port * 3);
+
+ return swsci(dev, SWSCI_SBCB_DISPLAY_POWER_STATE, parm, NULL);
+}
+
static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
{
struct drm_i915_private *dev_priv = dev->dev_private;
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/6] drm/i915: add plumbing for SWSCI
SWSCI is a driver to bios call interface.
This checks for SWSCI availability and bios requested callbacks, and
filters out any calls that shouldn't happen. This way the callers don't
need to do the checks all over the place.
v2: silence some checkpatch nagging
v3: set PCI_SWSCI bit 0 to trigger interrupt (Mengdong Lin)
v4: remove an extra #define (Jesse)
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/i915_drv.h |1 +
drivers/gpu/drm/i915/intel_opregion.c | 121 -
2 files changed, 121 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 84b95b1..adc2f46 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -225,6 +225,7 @@ struct intel_opregion {
struct opregion_header __iomem *header;
struct opregion_acpi __iomem *acpi;
struct opregion_swsci __iomem *swsci;
+ u32 swsci_requested_callbacks;
struct opregion_asle __iomem *asle;
void __iomem *vbt;
u32 __iomem *lid_state;
diff --git a/drivers/gpu/drm/i915/intel_opregion.c
b/drivers/gpu/drm/i915/intel_opregion.c
index cfb8fb6..a3558ae 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -151,6 +151,51 @@ struct opregion_asle {
#define ASLE_CBLV_VALID (1<<31)
+/* Software System Control Interrupt (SWSCI) */
+#define SWSCI_SCIC_INDICATOR (1 << 0)
+#define SWSCI_SCIC_MAIN_FUNCTION_SHIFT 1
+#define SWSCI_SCIC_MAIN_FUNCTION_MASK (0xf << 1)
+#define SWSCI_SCIC_SUB_FUNCTION_SHIFT 8
+#define SWSCI_SCIC_SUB_FUNCTION_MASK (0x7f << 8)
+#define SWSCI_SCIC_EXIT_STATUS_SHIFT 5
+#define SWSCI_SCIC_EXIT_STATUS_MASK(7 << 5)
+#define SWSCI_SCIC_EXIT_STATUS_SUCCESS 1
+
+#define SWSCI_FUNCTION_CODE(main, sub) \
+ ((main) << SWSCI_SCIC_MAIN_FUNCTION_SHIFT | \
+(sub) << SWSCI_SCIC_SUB_FUNCTION_SHIFT)
+
+/* SWSCI: Get BIOS Data (GBDA) */
+#define SWSCI_GBDA 4
+#define SWSCI_GBDA_SUPPORTED_CALLS SWSCI_FUNCTION_CODE(SWSCI_GBDA, 0)
+#define SWSCI_GBDA_REQUESTED_CALLBACKS SWSCI_FUNCTION_CODE(SWSCI_GBDA, 1)
+#define SWSCI_GBDA_BOOT_DISPLAY_PREF SWSCI_FUNCTION_CODE(SWSCI_GBDA, 4)
+#define SWSCI_GBDA_PANEL_DETAILS SWSCI_FUNCTION_CODE(SWSCI_GBDA, 5)
+#define SWSCI_GBDA_TV_STANDARD SWSCI_FUNCTION_CODE(SWSCI_GBDA, 6)
+#define SWSCI_GBDA_INTERNAL_GRAPHICS SWSCI_FUNCTION_CODE(SWSCI_GBDA, 7)
+#define SWSCI_GBDA_SPREAD_SPECTRUM SWSCI_FUNCTION_CODE(SWSCI_GBDA, 10)
+
+/* SWSCI: System BIOS Callbacks (SBCB) */
+#define SWSCI_SBCB 6
+#define SWSCI_SBCB_SUPPORTED_CALLBACKS SWSCI_FUNCTION_CODE(SWSCI_SBCB, 0)
+#define SWSCI_SBCB_INIT_COMPLETION SWSCI_FUNCTION_CODE(SWSCI_SBCB, 1)
+#define SWSCI_SBCB_PRE_HIRES_SET_MODE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 3)
+#define SWSCI_SBCB_POST_HIRES_SET_MODE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 4)
+#define SWSCI_SBCB_DISPLAY_SWITCH SWSCI_FUNCTION_CODE(SWSCI_SBCB, 5)
+#define SWSCI_SBCB_SET_TV_FORMAT SWSCI_FUNCTION_CODE(SWSCI_SBCB, 6)
+#define SWSCI_SBCB_ADAPTER_POWER_STATE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 7)
+#define SWSCI_SBCB_DISPLAY_POWER_STATE SWSCI_FUNCTION_CODE(SWSCI_SBCB, 8)
+#define SWSCI_SBCB_SET_BOOT_DISPLAYSWSCI_FUNCTION_CODE(SWSCI_SBCB, 9)
+#define SWSCI_SBCB_SET_INTERNAL_GFXSWSCI_FUNCTION_CODE(SWSCI_SBCB, 11)
+#define SWSCI_SBCB_POST_HIRES_TO_DOS_FSSWSCI_FUNCTION_CODE(SWSCI_SBCB,
16)
+#define SWSCI_SBCB_SUSPEND_RESUME SWSCI_FUNCTION_CODE(SWSCI_SBCB, 17)
+#define SWSCI_SBCB_SET_SPREAD_SPECTRUM SWSCI_FUNCTION_CODE(SWSCI_SBCB, 18)
+#define SWSCI_SBCB_POST_VBE_PM SWSCI_FUNCTION_CODE(SWSCI_SBCB, 19)
+#define SWSCI_SBCB_ENABLE_DISABLE_AUDIOSWSCI_FUNCTION_CODE(SWSCI_SBCB,
21)
+
+#define PCI_SWSCI 0xe8
+#define PCI_SWSCI_IRQ (1 << 0)
+
#define ACPI_OTHER_OUTPUT (0<<8)
#define ACPI_VGA_OUTPUT (1<<8)
#define ACPI_TV_OUTPUT (2<<8)
@@ -158,6 +203,76 @@ struct opregion_asle {
#define ACPI_LVDS_OUTPUT (4<<8)
#ifdef CONFIG_ACPI
+static int swsci(struct drm_device *dev, u32 function, u32 parm, u32 *parm_out)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct opregion_swsci __iomem *swsci = dev_priv->opregion.swsci;
+ u32 main_function, sub_function, scic;
+ u16 pci_swsci;
+
+ if (!swsci)
+ return 0;
+
+ main_function = (function & SWSCI_SCIC_MAIN_FUNCTION_MASK) >>
+ SWSCI_SCIC_MAIN_FUNCTION_SHIFT;
+ sub_function = (function & SWSCI_SCIC_SUB_FUNCTION_MASK) >>
+ SWSCI_SCIC_SUB_FUNCTION_SHIFT;
+
+ if (main_function == SWSCI_SBCB && sub_function != 0) {
+ /*
+* SBCB sub-function codes correspond to the bits in requested
+* callbacks, except for bit 0 which is reserved. The driver
+* must not call interfaces that are not specifically requested
+* by th
[Intel-gfx] [PATCH 1/6] drm/i915: expose intel_ddi_get_encoder_port()
In preparation for followup work.
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/i915/intel_ddi.c |2 +-
drivers/gpu/drm/i915/intel_drv.h |1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 63aca49..060ea50 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -58,7 +58,7 @@ static const u32 hsw_ddi_translations_fdi[] = {
0x00FF, 0x00040006 /* HDMI parameters */
};
-static enum port intel_ddi_get_encoder_port(struct intel_encoder
*intel_encoder)
+enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
{
struct drm_encoder *encoder = &intel_encoder->base;
int type = intel_encoder->type;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1760808..3ab1a52 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -707,6 +707,7 @@ extern void intel_write_eld(struct drm_encoder *encoder,
extern void intel_prepare_ddi(struct drm_device *dev);
extern void hsw_fdi_link_train(struct drm_crtc *crtc);
extern void intel_ddi_init(struct drm_device *dev, enum port port);
+extern enum port intel_ddi_get_encoder_port(struct intel_encoder
*intel_encoder);
/* For use by IVB LP watermark workaround in intel_sprite.c */
extern void intel_update_watermarks(struct drm_device *dev);
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/6] drm/i915: BIOS display/adapter power state notifications
Hi all, here are some patches for letting the BIOS do some further power savings on HSW, now based on intel-drm-nightly with Paulo's PC8+ work. Patches 1-4 are the glue code to wrap the BIOS SWSCI calls into a few sensible function calls. Patches 5-6 are for reference only, and I presume they will have to be tweaked based on testing. Where exactly to do the calls? What parameters are to be used in the end? Possibly calls need to be added on the suspend/resume paths too. I don't have the setup to do the testing and measurements, but I hope these will enable others. BR, Jani. Jani Nikula (6): drm/i915: expose intel_ddi_get_encoder_port() drm/i915: add plumbing for SWSCI drm/i915: add opregion function to notify bios of encoder enable/disable drm/i915: add opregion function to notify bios of adapter power state DRAFT: drm/i915: do display power state notification on crtc enable/disable DRAFT: drm/i915: do adapter power state notification on PC8+ enable/disable drivers/gpu/drm/i915/i915_drv.h | 12 ++ drivers/gpu/drm/i915/intel_ddi.c |2 +- drivers/gpu/drm/i915/intel_display.c | 12 +- drivers/gpu/drm/i915/intel_drv.h |1 + drivers/gpu/drm/i915/intel_opregion.c | 200 - 5 files changed, 223 insertions(+), 4 deletions(-) -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix context size calculation on SNB/IVB/VLV
On Thu, Aug 22, 2013 at 11:30:55AM -0700, Ben Widawsky wrote: > On Thu, Aug 22, 2013 at 07:23:13PM +0300, ville.syrj...@linux.intel.com wrote: > > From: Ville Syrjälä > > > > All the different context sizes reported in the CXT_SIZE register > > aren't meant to be simply added together. > > > > While BSpec is somewhat unclear on the topic of the actual context > > size, empirical tests have now revealed the truth. So let's add a > > big fat comment to remind people how it all works. > > By the way. I've done some digging. I believe (75% certain) pre-HSW, > every context save writes the entire data. So if you wanted to set some > pattern and see what HW actually overwrites, it should be doable. HSW+ > though we can't do that. So I did this test on SNB and IVB and it confirms my earlier findings. This experiment also showed that while SNB leaves room for the ring context, it doesn't actually write to it. -- 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] Fix for the i915_vma_unbind() fix.
We need to move the bogus warn out of the way and only do the
vma->vma_link decoupling before destroying the node.
Fixes gem_evict_everything, gem_evict_alignment
Squash in with Daniel's fixup to the fix -- I expect that he has already
done so...
Signed-off-by: Chris Wilson
---
drivers/gpu/drm/i915/i915_gem.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5788e9d..744f9a6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2725,6 +2725,9 @@ int i915_vma_unbind(struct i915_vma *vma)
if (list_empty(&vma->vma_link))
return 0;
+/* NB: Until we have real VMAs there will only ever be one */
+ WARN_ON(!list_is_singular(&obj->vma_list));
+
if (!drm_mm_node_allocated(&vma->node))
goto destroy;
@@ -2771,8 +2774,7 @@ destroy:
/* Since the unbound list is global, only move to that list if
* no more VMAs exist.
-* NB: Until we have real VMAs there will only ever be one */
- WARN_ON(!list_empty(&obj->vma_list));
+*/
if (list_empty(&obj->vma_list))
list_move_tail(&obj->global_list, &dev_priv->mm.unbound_list);
@@ -4279,12 +4281,12 @@ i915_gem_obj_lookup_or_create_vma(struct
drm_i915_gem_object *obj,
void i915_gem_vma_destroy(struct i915_vma *vma)
{
WARN_ON(vma->node.allocated);
- list_del(&vma->vma_link);
/* Keep the vma as a placeholder in the execbuffer reservation lists */
if (!list_empty(&vma->exec_list))
return;
+ list_del(&vma->vma_link);
kfree(vma);
}
--
1.8.4.rc3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] linux-next: Tree for Aug 21 [ screen corruption in graphical mode ]
On Fri, Aug 23, 2013 at 10:34 AM, Chris Wilson wrote: > On Fri, Aug 23, 2013 at 10:04:37AM +0200, Sedat Dilek wrote: >> On Fri, Aug 23, 2013 at 9:55 AM, Sedat Dilek wrote: >> > On Thu, Aug 22, 2013 at 1:32 PM, Daniel Vetter >> > wrote: >> >> On Thu, Aug 22, 2013 at 1:30 PM, Daniel Vetter >> >> wrote: >> >>> On Thu, Aug 22, 2013 at 1:13 PM, Sedat Dilek >> >>> wrote: >> dmesg (a lot of traces) and kernel-config attached. >> >> UXA causes still screen corruption. >> >>> >> >>> Hm, was only a slim chance that this patch would fix anything - I >> >>> think you'd always see an oops when you'd hit this bug instead of just >> >>> a bit of corruption. >> >> >> >> Ok, I think it's time to throw in the towel a bit. I've dropped >> >> >> >> >> >> commit d46f1c3f1372e3a72fab97c60480aa4a1084387f >> >> Author: Chris Wilson >> >> Date: Thu Aug 8 14:41:06 2013 +0100 >> >> >> >> drm/i915: Allow the GPU to cache stolen memory > > Hmm, wrong patch. Unless you have a good reason, you just want to drop > the ringbuffers in stolen. > >> >> from my queue. I guess we can retry for 3.13 again. >> > >> > I am sorry to keep someone's work to be delayed, really. >> > I would have liked to see this fixed (and I have spent some time on it). > > It's just a minor memory optimization (reclaiming less than a megabyte > of system memory). > >> > Which patches did you exactly drop? >> > >> >> Sorry for bombing you with question... >> >> I am trying latest Linus-tree HEAD with the drm-intel-nightly I made >> my last testings. >> >> Are any of these TLB / x86-get_unmapped_area fixes of interested... >> has any effects on the reported issue? > > It should not. Of concern is how the GPU views the world which has its > own independent set of TLBs and mapping tables - and access to those > should always be uncached from the CPU's perspective. > s Linux-v3.11-rc6-76-g6a7492a with my last d-i-n on top shows still the same issue with UXA. So, this is unrelated. >> I still wonder what is the root-cause... >> I mean if SNA is OK but UXA not and Linux graphics stack is that complex. >> ( Can't say if user-space like unity isn't involved... ). > > All that userspace can affect here is the timing and inital contents of > the framebuffer, everything else is controlled by the kernel. All the > testing we have done so far imply that the kernel's view of the machine > state is consistent with our expectations, but the display is doing > something inexplicable. I checked for a new BIOS, but version 13XK is the last available. If I start in text-mode and then run from my VT-1 lightdm service manually I see this screen/display corruption. On a restart of lightdm everything is fine. How can I check if my greeter and/or unity(-2d) is the culprit? AFAICS I have here E17 for testing. I made no benchmark with the (new) d-i-n w/ dropped patches. Lemme see if I find some time. - Sedat - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PULL] drm-intel-fixes
Argh, forgotten to cc lists! On Fri, Aug 23, 2013 at 10:42:37AM +0200, Daniel Vetter wrote: > Hi Dave, > > Just one patch that soaked for quite a bit to fix a resume issue, > resulting in gpu hangs (or worse) due to tlb containing garbage. > > Cheers, Daniel > > > The following changes since commit 63b66e5ba54b15a6592be00555d762db6db739ce: > > drm/i915: Don't deref pipe->cpu_transcoder in the hangcheck code > (2013-08-14 20:26:49 +0200) > > are available in the git repository at: > > git://people.freedesktop.org/~danvet/drm-intel > tags/drm-intel-fixes-2013-08-23 > > for you to fetch changes up to 884020bf3d2a3787a1cc6df902e98e0eec60330b: > > drm/i915: Invalidate TLBs for the rings after a reset (2013-08-18 19:37:41 > +0200) > > > Chris Wilson (1): > drm/i915: Invalidate TLBs for the rings after a reset > > drivers/gpu/drm/i915/i915_reg.h | 2 ++ > drivers/gpu/drm/i915/intel_ringbuffer.c | 12 > 2 files changed, 14 insertions(+) > -- > 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] drm/i915: Fix __wait_seqno to use true infinite timeouts
On Fri, Aug 23, 2013 at 10:06 AM, Chris Wilson wrote: >> Yeah, I think this approach should work. A few comments: >> - I think we need a debugfs file to shut the safety quirk off - when >> testing on a machine where we actually miss interrupts it might be >> usful to get the warning output every time. > > I did consider a modparam. Exposing it would indeed necessitate some > protection against concurrent modificatin. > >> - I'd go for a new bool dev_priv->unreliable_seqno_signalling or so to >> avoid any races due to the rmw cycle you now do on dev_priv->quirks. > > There isn't a race during writing, as hangcheck should never be run > concurrently (or at least any concurrent calls filtered out at the start > of the function). The read side is inherently racey. It's more that thus far we've used dev_priv->quirks only for stuff which never changes, now we have a quirk which gets only set at runtime. It just feels conceptually wrong ;-) And if we add a 2nd such quirk it'll break a bit, hence why I'd prefer a distinct piece of state tracking >> - I'd have opted for a faster timeout of the fake irq, but one that rearms. > > Whoops, that was a mistake. The intention was to run at 100Hz, do you > want even faster? We could switch to a hrtimer and kill two birds with > one stone (as timer is singleshot only). I'd simply go with a 1 jiffies rearming timer. >> Also I'd love to be able to test all this (both the missed irq >> detection stuff and the fake irq) but I don't have a good idea right >> now ... So I guess we need to again hope that it won't break too >> quickly (since it eventually will break again). > > Disable the call to ring->get_irq. Perhaps the high word of > dev_priv->stop_rings? Yeah, something like stop_ring_irqs or so could work, maybe by wrapping the wake_up_all calls into a little helper like wake_up_ring which noops if the respective bit is set in stop_ring_irqs. But I'm not sure whether this is worth the fuzz. Otoh we've just proven that for untested code the question isn't if it breaks, but when ... -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] linux-next: Tree for Aug 21 [ screen corruption in graphical mode ]
On Fri, Aug 23, 2013 at 10:04:37AM +0200, Sedat Dilek wrote: > On Fri, Aug 23, 2013 at 9:55 AM, Sedat Dilek wrote: > > On Thu, Aug 22, 2013 at 1:32 PM, Daniel Vetter > > wrote: > >> On Thu, Aug 22, 2013 at 1:30 PM, Daniel Vetter > >> wrote: > >>> On Thu, Aug 22, 2013 at 1:13 PM, Sedat Dilek > >>> wrote: > dmesg (a lot of traces) and kernel-config attached. > > UXA causes still screen corruption. > >>> > >>> Hm, was only a slim chance that this patch would fix anything - I > >>> think you'd always see an oops when you'd hit this bug instead of just > >>> a bit of corruption. > >> > >> Ok, I think it's time to throw in the towel a bit. I've dropped > >> > >> > >> commit d46f1c3f1372e3a72fab97c60480aa4a1084387f > >> Author: Chris Wilson > >> Date: Thu Aug 8 14:41:06 2013 +0100 > >> > >> drm/i915: Allow the GPU to cache stolen memory Hmm, wrong patch. Unless you have a good reason, you just want to drop the ringbuffers in stolen. > >> from my queue. I guess we can retry for 3.13 again. > > > > I am sorry to keep someone's work to be delayed, really. > > I would have liked to see this fixed (and I have spent some time on it). It's just a minor memory optimization (reclaiming less than a megabyte of system memory). > > Which patches did you exactly drop? > > > > Sorry for bombing you with question... > > I am trying latest Linus-tree HEAD with the drm-intel-nightly I made > my last testings. > > Are any of these TLB / x86-get_unmapped_area fixes of interested... > has any effects on the reported issue? It should not. Of concern is how the GPU views the world which has its own independent set of TLBs and mapping tables - and access to those should always be uncached from the CPU's perspective. > I still wonder what is the root-cause... > I mean if SNA is OK but UXA not and Linux graphics stack is that complex. > ( Can't say if user-space like unity isn't involved... ). All that userspace can affect here is the timing and inital contents of the framebuffer, everything else is controlled by the kernel. All the testing we have done so far imply that the kernel's view of the machine state is consistent with our expectations, but the display is doing something inexplicable. -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: Fix __wait_seqno to use true infinite timeouts
On Fri, Aug 23, 2013 at 09:27:42AM +0200, Daniel Vetter wrote: > On Fri, Aug 23, 2013 at 3:05 AM, Chris Wilson > wrote: > > Rather than continue to fix up the timeouts to work around the interface > > impedence in wait_event_*(), open code the combination of > > wait_event[_interruptible][_timeout]. And note the code size reduction, > > and dare say readability?, in doing so.. > > > > v2: In order to satisfy the debug requirement of logging missed > > interrupts with the real world requirments of making machines work even > > if interrupts are hosed, we revert to polling after detecting a missed > > interrupt. > > > > Signed-off-by: Chris Wilson > > > Yeah, I think this approach should work. A few comments: > - I think we need a debugfs file to shut the safety quirk off - when > testing on a machine where we actually miss interrupts it might be > usful to get the warning output every time. I did consider a modparam. Exposing it would indeed necessitate some protection against concurrent modificatin. > - I'd go for a new bool dev_priv->unreliable_seqno_signalling or so to > avoid any races due to the rmw cycle you now do on dev_priv->quirks. There isn't a race during writing, as hangcheck should never be run concurrently (or at least any concurrent calls filtered out at the start of the function). The read side is inherently racey. > - I'd have opted for a faster timeout of the fake irq, but one that rearms. Whoops, that was a mistake. The intention was to run at 100Hz, do you want even faster? We could switch to a hrtimer and kill two birds with one stone (as timer is singleshot only). > Also I'd love to be able to test all this (both the missed irq > detection stuff and the fake irq) but I don't have a good idea right > now ... So I guess we need to again hope that it won't break too > quickly (since it eventually will break again). Disable the call to ring->get_irq. Perhaps the high word of dev_priv->stop_rings? -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] linux-next: Tree for Aug 21 [ screen corruption in graphical mode ]
On Fri, Aug 23, 2013 at 9:55 AM, Sedat Dilek wrote: > On Thu, Aug 22, 2013 at 1:32 PM, Daniel Vetter wrote: >> On Thu, Aug 22, 2013 at 1:30 PM, Daniel Vetter >> wrote: >>> On Thu, Aug 22, 2013 at 1:13 PM, Sedat Dilek wrote: dmesg (a lot of traces) and kernel-config attached. UXA causes still screen corruption. >>> >>> Hm, was only a slim chance that this patch would fix anything - I >>> think you'd always see an oops when you'd hit this bug instead of just >>> a bit of corruption. >> >> Ok, I think it's time to throw in the towel a bit. I've dropped >> >> >> commit d46f1c3f1372e3a72fab97c60480aa4a1084387f >> Author: Chris Wilson >> Date: Thu Aug 8 14:41:06 2013 +0100 >> >> drm/i915: Allow the GPU to cache stolen memory >> >> from my queue. I guess we can retry for 3.13 again. > > I am sorry to keep someone's work to be delayed, really. > I would have liked to see this fixed (and I have spent some time on it). > > Which patches did you exactly drop? > Sorry for bombing you with question... I am trying latest Linus-tree HEAD with the drm-intel-nightly I made my last testings. Are any of these TLB / x86-get_unmapped_area fixes of interested... has any effects on the reported issue? I still wonder what is the root-cause... I mean if SNA is OK but UXA not and Linux graphics stack is that complex. ( Can't say if user-space like unity isn't involved... ). - Sedat - [1] Fix TLB gather virtual address range invalidation corner cases http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/patch/?id=2b047252d087be7f2ba088b4933cd904f92e6fce [2] Revert "x86 get_unmapped_area(): use proper mmap base for bottom-up direction" http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/patch/?id=5ea80f76a56605a190a7ea16846c82aa63dbd0aa [3] x86 get_unmapped_area: Access mmap_legacy_base through mm_struct member http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/patch/?id=41aacc1eea645c99edbe8fbcf78a97dc9b862adc ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: try not to lose backlight CBLV precision
On 08/23/2013 03:50 PM, Jani Nikula wrote:
> ACPI has _BCM and _BQC methods to set and query the backlight
> brightness, respectively. The ACPI opregion has variables BCLP and CBLV
> to hold the requested and current backlight brightness, respectively.
>
> The BCLP variable has range 0..255 while the others have range
> 0..100. This means the _BCM method has to scale the brightness for BCLP,
> and the gfx driver has to scale the requested value back for CBLV. If
> the _BQC method uses the CBLV variable (apparently some implementations
> do, some don't) for current backlight level reporting, there's room for
> rounding errors.
>
> Use DIV_ROUND_UP for scaling back to CBLV to get back to the same values
> that were passed to _BCM, presuming the _BCM simply uses bclp = (in *
> 255) / 100 for scaling to BCLP.
>
> Reference: https://gist.github.com/aaronlu/6314920
> Reported-by: Aaron Lu
> Signed-off-by: Jani Nikula
Reviewed-by: Aaron Lu
Thanks,
Aaron
>
> ---
>
> All of https://gist.github.com/aaronlu/6314920 included here for
> reference:
>
> A typical ASL code for the backlight control method _BCM with Intel
> graphics card is as follows:
>
> Method (_BCM, 1, NotSerialized)
> {
> If (BRNC)
> {
> AINT (One, Arg0)
> }
> Else
> {
> ^^^LPCB.EC0.STBR ()
> }
> }
> _BCM method takes one param: the target brightness level in the range
> of 0-100. The BRNC variable is set if bit2 of _DOS's param is set,
> which we do for Win8 systems now, so AINT will be executed.
>
> And the simplified ASL code for AINT on backlight control is:
>
> Method (AINT, 2, Serialized)
> {
> If (LEqual (Arg0, One))
> {
> Store (Divide (Multiply (Arg1, 0xFF), 0x64, ), BCLP)
> Or (BCLP, 0x8000, BCLP)
> Store (0x02, ASLC)
> }
> }
>
> The ASLC/BCLP are variables declared in IGD operation region. BCLP is
> used to store the target brightness level in the range of 0-255. Due to
> the mismatch of the level range in _BCM and BCLP, a convert is done here
> for BCLP. The setting of the ASLC variable will trigger interrupt of the
> graphics card and the GPU driver will find out this is due to IGD operation
> region and will handle the irq accordingly. In backlight case, it will
> set backlight level in the GPU driver according to the value of BCLP.
>
> So the setting of backlight is actually done in GPU driver, even though
> it is triggered through firmware's interface. A side note is, there are
> ASL implementations that would trigger the SMI handler on backlight
> control and that would also result in GPU driver's irq handler and GPU
> driver will handle backlight setting then.
>
> So this is how to make use of IGD operation region to do the backlight
> brightness level control.
>
> There is a problem related to _BQC though on some firmware implementation.
> _BQC is a control method provided by firmware to tell which backlight
> level the firmware thinks the device is in. The broken implementation is:
>
> Method (_BQC, 0, NotSerialized) // _BQC: Brightness Query Current
> {
> If (LGreaterEqual (MSOS (), OSW8))
> {
> And (CBLV, 0x7FFF, Local0)
> Return (Local0)
> }
> }
>
> CBLV is a variable in IGD operation region, used to represent the
> current brightness level in the range of 0-100 and is updated by
> GPU driver everytime it is asked to set the backlight level.
>
> Say user wants to set target level to 8, then 8 will be converted to
> 20(8 * 255 / 100) for BCLP in AINT, then in GPU driver, 20 will be
> converted again to 7(20 * 100 / 255) for CBLV, so _BQC will return 7
> afterwards though user actually sets 8 in _BCM. But this doesn't happen
> for every level set through _BCM, for those values that do not lose
> precisions during the conversion back and forth like 20 are not affected.
> This needs to be remembered when enhancing the quirk logic of _BQC,
> unless we can somehow fix the problem.
>
> Some firmware doesn't have this problem as they simply store the target
> level user has requested in _BCM in a variable and then return that
> variable in _BQC, but then we probably do not need to evaluate _BQC at
> all since we also know what level the device should be in too in ACPI
> video module.
>
> PS: The above example ASL code is taken from a ASUS NV5Z system.
> ---
> drivers/gpu/drm/i915/intel_opregion.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c
> b/drivers/gpu/drm/i915/intel_opregion.c
> index cfb8fb6..119771f 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -173,7 +173,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32
> bclp)
> retur
Re: [Intel-gfx] linux-next: Tree for Aug 21 [ screen corruption in graphical mode ]
On Thu, Aug 22, 2013 at 1:32 PM, Daniel Vetter wrote: > On Thu, Aug 22, 2013 at 1:30 PM, Daniel Vetter wrote: >> On Thu, Aug 22, 2013 at 1:13 PM, Sedat Dilek wrote: >>> dmesg (a lot of traces) and kernel-config attached. >>> >>> UXA causes still screen corruption. >> >> Hm, was only a slim chance that this patch would fix anything - I >> think you'd always see an oops when you'd hit this bug instead of just >> a bit of corruption. > > Ok, I think it's time to throw in the towel a bit. I've dropped > > > commit d46f1c3f1372e3a72fab97c60480aa4a1084387f > Author: Chris Wilson > Date: Thu Aug 8 14:41:06 2013 +0100 > > drm/i915: Allow the GPU to cache stolen memory > > from my queue. I guess we can retry for 3.13 again. I am sorry to keep someone's work to be delayed, really. I would have liked to see this fixed (and I have spent some time on it). Which patches did you exactly drop? - Sedat - ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: try not to lose backlight CBLV precision
ACPI has _BCM and _BQC methods to set and query the backlight
brightness, respectively. The ACPI opregion has variables BCLP and CBLV
to hold the requested and current backlight brightness, respectively.
The BCLP variable has range 0..255 while the others have range
0..100. This means the _BCM method has to scale the brightness for BCLP,
and the gfx driver has to scale the requested value back for CBLV. If
the _BQC method uses the CBLV variable (apparently some implementations
do, some don't) for current backlight level reporting, there's room for
rounding errors.
Use DIV_ROUND_UP for scaling back to CBLV to get back to the same values
that were passed to _BCM, presuming the _BCM simply uses bclp = (in *
255) / 100 for scaling to BCLP.
Reference: https://gist.github.com/aaronlu/6314920
Reported-by: Aaron Lu
Signed-off-by: Jani Nikula
---
All of https://gist.github.com/aaronlu/6314920 included here for
reference:
A typical ASL code for the backlight control method _BCM with Intel
graphics card is as follows:
Method (_BCM, 1, NotSerialized)
{
If (BRNC)
{
AINT (One, Arg0)
}
Else
{
^^^LPCB.EC0.STBR ()
}
}
_BCM method takes one param: the target brightness level in the range
of 0-100. The BRNC variable is set if bit2 of _DOS's param is set,
which we do for Win8 systems now, so AINT will be executed.
And the simplified ASL code for AINT on backlight control is:
Method (AINT, 2, Serialized)
{
If (LEqual (Arg0, One))
{
Store (Divide (Multiply (Arg1, 0xFF), 0x64, ), BCLP)
Or (BCLP, 0x8000, BCLP)
Store (0x02, ASLC)
}
}
The ASLC/BCLP are variables declared in IGD operation region. BCLP is
used to store the target brightness level in the range of 0-255. Due to
the mismatch of the level range in _BCM and BCLP, a convert is done here
for BCLP. The setting of the ASLC variable will trigger interrupt of the
graphics card and the GPU driver will find out this is due to IGD operation
region and will handle the irq accordingly. In backlight case, it will
set backlight level in the GPU driver according to the value of BCLP.
So the setting of backlight is actually done in GPU driver, even though
it is triggered through firmware's interface. A side note is, there are
ASL implementations that would trigger the SMI handler on backlight
control and that would also result in GPU driver's irq handler and GPU
driver will handle backlight setting then.
So this is how to make use of IGD operation region to do the backlight
brightness level control.
There is a problem related to _BQC though on some firmware implementation.
_BQC is a control method provided by firmware to tell which backlight
level the firmware thinks the device is in. The broken implementation is:
Method (_BQC, 0, NotSerialized) // _BQC: Brightness Query Current
{
If (LGreaterEqual (MSOS (), OSW8))
{
And (CBLV, 0x7FFF, Local0)
Return (Local0)
}
}
CBLV is a variable in IGD operation region, used to represent the
current brightness level in the range of 0-100 and is updated by
GPU driver everytime it is asked to set the backlight level.
Say user wants to set target level to 8, then 8 will be converted to
20(8 * 255 / 100) for BCLP in AINT, then in GPU driver, 20 will be
converted again to 7(20 * 100 / 255) for CBLV, so _BQC will return 7
afterwards though user actually sets 8 in _BCM. But this doesn't happen
for every level set through _BCM, for those values that do not lose
precisions during the conversion back and forth like 20 are not affected.
This needs to be remembered when enhancing the quirk logic of _BQC,
unless we can somehow fix the problem.
Some firmware doesn't have this problem as they simply store the target
level user has requested in _BCM in a variable and then return that
variable in _BQC, but then we probably do not need to evaluate _BQC at
all since we also know what level the device should be in too in ACPI
video module.
PS: The above example ASL code is taken from a ASUS NV5Z system.
---
drivers/gpu/drm/i915/intel_opregion.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_opregion.c
b/drivers/gpu/drm/i915/intel_opregion.c
index cfb8fb6..119771f 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -173,7 +173,7 @@ static u32 asle_set_backlight(struct drm_device *dev, u32
bclp)
return ASLE_BACKLIGHT_FAILED;
intel_panel_set_backlight(dev, bclp, 255);
- iowrite32((bclp*0x64)/0xff | ASLE_CBLV_VALID, &asle->cblv);
+ iowrite32(DIV_ROUND_UP(bclp * 100, 255) | ASLE_CBLV_VALID, &asle->cblv);
return 0;
}
--
1.7.9.5
___
In
Re: [Intel-gfx] [PATCH] drm/i915: Fix __wait_seqno to use true infinite timeouts
On Fri, Aug 23, 2013 at 3:05 AM, Chris Wilson wrote: > Rather than continue to fix up the timeouts to work around the interface > impedence in wait_event_*(), open code the combination of > wait_event[_interruptible][_timeout]. And note the code size reduction, > and dare say readability?, in doing so.. > > v2: In order to satisfy the debug requirement of logging missed > interrupts with the real world requirments of making machines work even > if interrupts are hosed, we revert to polling after detecting a missed > interrupt. > > Signed-off-by: Chris Wilson Yeah, I think this approach should work. A few comments: - I think we need a debugfs file to shut the safety quirk off - when testing on a machine where we actually miss interrupts it might be usful to get the warning output every time. - I'd go for a new bool dev_priv->unreliable_seqno_signalling or so to avoid any races due to the rmw cycle you now do on dev_priv->quirks. - I'd have opted for a faster timeout of the fake irq, but one that rearms. Also I'd love to be able to test all this (both the missed irq detection stuff and the fake irq) but I don't have a good idea right now ... So I guess we need to again hope that it won't break too quickly (since it eventually will break again). -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
