[PATCH 00/10] drm/i915: Fixes from my attempt at running igt on gen2

[PATCH 00/10] drm/i915: Fixes from my attempt at running igt on gen2

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

It's been a while since I last ran igt on gen2, so I figured I'd
give it a shot. 855 had some failures, 830 no longer worked at
all. So I went ahead and fixed them, and here's the result.

The first three patches are not even gen2 specific bugs I caught
with this effort. The rest is for gen2.

I have some fixes for igt as well, which I'll post separately.

The good news is that with these patches (and the igt fixes) my
855 completes a full kms_flip run without failures, and the BAT
set has only one failure (gem_render_tiled_blits). 830 is fairly
good too, but it does have a lot of underruns and pipe_assert()
dmesg warnings. Lot of those are due to the pipe enable quirks
since we implement those quite haphazardly.

The series is available here:
git://github.com/vsyrjala/linux.git gen2_igt_fixes

Ville Syrjälä (10):
  drm/i915: Release mmaps on partial ggtt vma unbind
  drm/i915: Cleanup phys status page too
  drm/i915: Write out crc frame counts in hex
  drm/i915: Wait for pipe to start before sampling vblank timestamps on
    gen2
  drm/i915: Use drm_vblank_count() on gen2 for crc frame count
  drm/i915: Enable vblank_disable_immediate on gen2
  drm/i915: Allow 27 bytes child_dev for VBT <109
  drm/i915: Expect child dev size of 22 bytes for VBT < 106
  drm/i915: Reject < 8 byte batches on 830/845
  drm/i915: Use MI_BATCH_BUFFER_START on 830/845

 drivers/gpu/drm/i915/i915_debugfs.c        | 13 ++++++++++++-
 drivers/gpu/drm/i915/i915_gem.c            |  3 +++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  3 +++
 drivers/gpu/drm/i915/i915_irq.c            | 14 +++++---------
 drivers/gpu/drm/i915/intel_bios.c          | 21 ++++++++++++--------
 drivers/gpu/drm/i915/intel_display.c       | 11 +++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.c    | 31 +++++++++++++++++++++++-------
 7 files changed, 71 insertions(+), 25 deletions(-)

-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/10] drm/i915: Release mmaps on partial ggtt vma unbind

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

When a partial ggtt vma gets evicted, we need to zap any CPU
mapping to said vma as well. Currently we zap the mappings
only when the normal gtt vma gets evicted, but for partial
vmas we leave behind stale CPU mappins. And so, if something
else gets bound into the same gtt address range, any
userspace access into the relevant virtual addresses will
go astray.

I didn't find anything really suitable in the mm code to zap
just the needed mappings (we'd need to know the right CPU
side mm and vma etc.), so let's just call i915_gem_release_mmap()
for now.

Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Testcase: igt/gem_mmap_gtt
Fixes: c5ad54c ("drm/i915: Use partial view in mmap fault handler")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 66b170598ae6..c29b929f796c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3264,6 +3264,9 @@ static int __i915_vma_unbind(struct i915_vma *vma, bool wait)
 		ret = i915_gem_object_put_fence(obj);
 		if (ret)
 			return ret;
+	} else if (i915_is_ggtt(vma->vm) &&
+		   vma->ggtt_view.type == I915_GGTT_VIEW_PARTIAL) {
+		i915_gem_release_mmap(obj);
 	}
 
 	trace_i915_vma_unbind(vma);
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/10] drm/i915: Release mmaps on partial ggtt vma unbind

[Chris Wilson]

On Mon, Dec 14, 2015 at 06:23:40PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> When a partial ggtt vma gets evicted, we need to zap any CPU
> mapping to said vma as well. Currently we zap the mappings
> only when the normal gtt vma gets evicted, but for partial
> vmas we leave behind stale CPU mappins. And so, if something
> else gets bound into the same gtt address range, any
> userspace access into the relevant virtual addresses will
> go astray.
> 
> I didn't find anything really suitable in the mm code to zap
> just the needed mappings (we'd need to know the right CPU
> side mm and vma etc.), so let's just call i915_gem_release_mmap()
> for now.
> 
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Testcase: igt/gem_mmap_gtt
> Fixes: c5ad54c ("drm/i915: Use partial view in mmap fault handler")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I fixed this by tracking vma->map_and_fenceable instead. We still kill
the entire object though. Note this is not enough to completely fix the
test case, so perhaps we wait until we can merge the series to do the
full fix?
-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: [PATCH 01/10] drm/i915: Release mmaps on partial ggtt vma unbind

[Ville Syrjälä]

On Mon, Dec 14, 2015 at 05:01:45PM +0000, Chris Wilson wrote:
> On Mon, Dec 14, 2015 at 06:23:40PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > When a partial ggtt vma gets evicted, we need to zap any CPU
> > mapping to said vma as well. Currently we zap the mappings
> > only when the normal gtt vma gets evicted, but for partial
> > vmas we leave behind stale CPU mappins. And so, if something
> > else gets bound into the same gtt address range, any
> > userspace access into the relevant virtual addresses will
> > go astray.
> > 
> > I didn't find anything really suitable in the mm code to zap
> > just the needed mappings (we'd need to know the right CPU
> > side mm and vma etc.), so let's just call i915_gem_release_mmap()
> > for now.
> > 
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Testcase: igt/gem_mmap_gtt
> > Fixes: c5ad54c ("drm/i915: Use partial view in mmap fault handler")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I fixed this by tracking vma->map_and_fenceable instead. We still kill
> the entire object though. Note this is not enough to completely fix the
> test case, so perhaps we wait until we can merge the series to do the
> full fix?

Sure, if you have something better in the pipeline I'm happy to drop
this.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/10] drm/i915: Cleanup phys status page too

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Restore the lost phys status page cleanup.

Fixes the following splat with DMA_API_DEBUG=y:

WARNING: CPU: 0 PID: 21615 at ../lib/dma-debug.c:974 dma_debug_device_change+0x190/0x1f0()
pci 0000:00:02.0: DMA-API: device driver has pending DMA allocations while released from device [count=1]
               One of leaked entries details: [device address=0x0000000023163000] [size=4096 bytes] [mapped with DMA_BIDIRECTIONAL] [mapped as coherent]
Modules linked in: i915(-) i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm sha256_generic hmac drbg ctr ccm sch_fq_codel binfmt_misc joydev mousedev arc4 ath5k iTCO_wdt mac80211 smsc_ircc2 ath snd_intel8x0m snd_intel8x0 snd_ac97_codec ac97_bus psmouse snd_pcm input_leds i2c_i801 pcspkr snd_timer cfg80211 snd soundcore i2c_core ehci_pci firewire_ohci ehci_hcd firewire_core lpc_ich 8139too rfkill crc_itu_t mfd_core mii usbcore rng_core intel_agp intel_gtt usb_common agpgart irda crc_ccitt fujitsu_laptop led_class parport_pc video parport evdev backlight
CPU: 0 PID: 21615 Comm: rmmod Tainted: G     U          4.4.0-rc4-mgm-ovl+ #4
Hardware name: FUJITSU SIEMENS LIFEBOOK S6120/FJNB16C, BIOS Version 1.26  05/10/2004
 e31a3de0 e31a3de0 e31a3d9c c128d4bd e31a3dd0 c1045a0c c15e00c4 e31a3dfc
 0000546f c15dfad2 000003ce c12b3740 000003ce c12b3740 00000000 00000001
 f61fb8a0 e31a3de8 c1045a83 00000009 e31a3de0 c15e00c4 e31a3dfc e31a3e4c
Call Trace:
 [<c128d4bd>] dump_stack+0x16/0x19
 [<c1045a0c>] warn_slowpath_common+0x8c/0xd0
 [<c12b3740>] ? dma_debug_device_change+0x190/0x1f0
 [<c12b3740>] ? dma_debug_device_change+0x190/0x1f0
 [<c1045a83>] warn_slowpath_fmt+0x33/0x40
 [<c12b3740>] dma_debug_device_change+0x190/0x1f0
 [<c1065499>] notifier_call_chain+0x59/0x70
 [<c10655af>] __blocking_notifier_call_chain+0x3f/0x80
 [<c106560f>] blocking_notifier_call_chain+0x1f/0x30
 [<c134cfb3>] __device_release_driver+0xc3/0xf0
 [<c134d0d7>] driver_detach+0x97/0xa0
 [<c134c440>] bus_remove_driver+0x40/0x90
 [<c134db18>] driver_unregister+0x28/0x60
 [<c1079e8c>] ? trace_hardirqs_on_caller+0x12c/0x1d0
 [<c12c0618>] pci_unregister_driver+0x18/0x80
 [<f83e96e7>] drm_pci_exit+0x87/0xb0 [drm]
 [<f8b3be2d>] i915_exit+0x1b/0x1ee [i915]
 [<c10b999c>] SyS_delete_module+0x14c/0x210
 [<c1079e8c>] ? trace_hardirqs_on_caller+0x12c/0x1d0
 [<c115a9bd>] ? ____fput+0xd/0x10
 [<c1002014>] do_fast_syscall_32+0xa4/0x450
 [<c149f6fa>] sysenter_past_esp+0x3b/0x5d
---[ end trace c2ecbc77760f10a0 ]---
Mapped at:
 [<c12b3183>] debug_dma_alloc_coherent+0x33/0x90
 [<f83e989c>] drm_pci_alloc+0x18c/0x1e0 [drm]
 [<f8acd59f>] intel_init_ring_buffer+0x2af/0x490 [i915]
 [<f8acd8b0>] intel_init_render_ring_buffer+0x130/0x750 [i915]
 [<f8aaea4e>] i915_gem_init_rings+0x1e/0x110 [i915]

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Fixes: 5c6c600 ("drm/i915: Remove DRI1 ring accessors and API")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index eefce9a3e9c8..0c005a3dc57f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1899,6 +1899,17 @@ i915_dispatch_execbuffer(struct drm_i915_gem_request *req,
 	return 0;
 }
 
+static void cleanup_phys_status_page(struct intel_engine_cs *ring)
+{
+	struct drm_i915_private *dev_priv = to_i915(ring->dev);
+
+	if (!dev_priv->status_page_dmah)
+		return;
+
+	drm_pci_free(ring->dev, dev_priv->status_page_dmah);
+	ring->status_page.page_addr = NULL;
+}
+
 static void cleanup_status_page(struct intel_engine_cs *ring)
 {
 	struct drm_i915_gem_object *obj;
@@ -1915,9 +1926,9 @@ static void cleanup_status_page(struct intel_engine_cs *ring)
 
 static int init_status_page(struct intel_engine_cs *ring)
 {
-	struct drm_i915_gem_object *obj;
+	struct drm_i915_gem_object *obj = ring->status_page.obj;
 
-	if ((obj = ring->status_page.obj) == NULL) {
+	if (obj == NULL) {
 		unsigned flags;
 		int ret;
 
@@ -2208,7 +2219,12 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
 	if (ring->cleanup)
 		ring->cleanup(ring);
 
-	cleanup_status_page(ring);
+	if (I915_NEED_GFX_HWS(ring->dev)) {
+		cleanup_status_page(ring);
+	} else {
+		BUG_ON(ring->id != RCS);
+		cleanup_phys_status_page(ring);
+	}
 
 	i915_cmd_parser_fini_ring(ring);
 	i915_gem_batch_pool_fini(&ring->batch_pool);
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/10] drm/i915: Cleanup phys status page too

[Chris Wilson]

On Mon, Dec 14, 2015 at 06:23:41PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Restore the lost phys status page cleanup.
> 
> Fixes the following splat with DMA_API_DEBUG=y:
> 
> WARNING: CPU: 0 PID: 21615 at ../lib/dma-debug.c:974 dma_debug_device_change+0x190/0x1f0()
> pci 0000:00:02.0: DMA-API: device driver has pending DMA allocations while released from device [count=1]
>                One of leaked entries details: [device address=0x0000000023163000] [size=4096 bytes] [mapped with DMA_BIDIRECTIONAL] [mapped as coherent]
> Modules linked in: i915(-) i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm sha256_generic hmac drbg ctr ccm sch_fq_codel binfmt_misc joydev mousedev arc4 ath5k iTCO_wdt mac80211 smsc_ircc2 ath snd_intel8x0m snd_intel8x0 snd_ac97_codec ac97_bus psmouse snd_pcm input_leds i2c_i801 pcspkr snd_timer cfg80211 snd soundcore i2c_core ehci_pci firewire_ohci ehci_hcd firewire_core lpc_ich 8139too rfkill crc_itu_t mfd_core mii usbcore rng_core intel_agp intel_gtt usb_common agpgart irda crc_ccitt fujitsu_laptop led_class parport_pc video parport evdev backlight
> CPU: 0 PID: 21615 Comm: rmmod Tainted: G     U          4.4.0-rc4-mgm-ovl+ #4
> Hardware name: FUJITSU SIEMENS LIFEBOOK S6120/FJNB16C, BIOS Version 1.26  05/10/2004
>  e31a3de0 e31a3de0 e31a3d9c c128d4bd e31a3dd0 c1045a0c c15e00c4 e31a3dfc
>  0000546f c15dfad2 000003ce c12b3740 000003ce c12b3740 00000000 00000001
>  f61fb8a0 e31a3de8 c1045a83 00000009 e31a3de0 c15e00c4 e31a3dfc e31a3e4c
> Call Trace:
>  [<c128d4bd>] dump_stack+0x16/0x19
>  [<c1045a0c>] warn_slowpath_common+0x8c/0xd0
>  [<c12b3740>] ? dma_debug_device_change+0x190/0x1f0
>  [<c12b3740>] ? dma_debug_device_change+0x190/0x1f0
>  [<c1045a83>] warn_slowpath_fmt+0x33/0x40
>  [<c12b3740>] dma_debug_device_change+0x190/0x1f0
>  [<c1065499>] notifier_call_chain+0x59/0x70
>  [<c10655af>] __blocking_notifier_call_chain+0x3f/0x80
>  [<c106560f>] blocking_notifier_call_chain+0x1f/0x30
>  [<c134cfb3>] __device_release_driver+0xc3/0xf0
>  [<c134d0d7>] driver_detach+0x97/0xa0
>  [<c134c440>] bus_remove_driver+0x40/0x90
>  [<c134db18>] driver_unregister+0x28/0x60
>  [<c1079e8c>] ? trace_hardirqs_on_caller+0x12c/0x1d0
>  [<c12c0618>] pci_unregister_driver+0x18/0x80
>  [<f83e96e7>] drm_pci_exit+0x87/0xb0 [drm]
>  [<f8b3be2d>] i915_exit+0x1b/0x1ee [i915]
>  [<c10b999c>] SyS_delete_module+0x14c/0x210
>  [<c1079e8c>] ? trace_hardirqs_on_caller+0x12c/0x1d0
>  [<c115a9bd>] ? ____fput+0xd/0x10
>  [<c1002014>] do_fast_syscall_32+0xa4/0x450
>  [<c149f6fa>] sysenter_past_esp+0x3b/0x5d
> ---[ end trace c2ecbc77760f10a0 ]---
> Mapped at:
>  [<c12b3183>] debug_dma_alloc_coherent+0x33/0x90
>  [<f83e989c>] drm_pci_alloc+0x18c/0x1e0 [drm]
>  [<f8acd59f>] intel_init_ring_buffer+0x2af/0x490 [i915]
>  [<f8acd8b0>] intel_init_render_ring_buffer+0x130/0x750 [i915]
>  [<f8aaea4e>] i915_gem_init_rings+0x1e/0x110 [i915]
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Fixes: 5c6c600 ("drm/i915: Remove DRI1 ring accessors and API")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Looks symmetrical (so I can't justify tyidying up the split :)
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 02/10] drm/i915: Cleanup phys status page too

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Restore the lost phys status page cleanup.

Fixes the following splat with DMA_API_DEBUG=y:

WARNING: CPU: 0 PID: 21615 at ../lib/dma-debug.c:974 dma_debug_device_change+0x190/0x1f0()
pci 0000:00:02.0: DMA-API: device driver has pending DMA allocations while released from device [count=1]
               One of leaked entries details: [device address=0x0000000023163000] [size=4096 bytes] [mapped with DMA_BIDIRECTIONAL] [mapped as coherent]
Modules linked in: i915(-) i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm sha256_generic hmac drbg ctr ccm sch_fq_codel binfmt_misc joydev mousedev arc4 ath5k iTCO_wdt mac80211 smsc_ircc2 ath snd_intel8x0m snd_intel8x0 snd_ac97_codec ac97_bus psmouse snd_pcm input_leds i2c_i801 pcspkr snd_timer cfg80211 snd soundcore i2c_core ehci_pci firewire_ohci ehci_hcd firewire_core lpc_ich 8139too rfkill crc_itu_t mfd_core mii usbcore rng_core intel_agp intel_gtt usb_common agpgart irda crc_ccitt fujitsu_laptop led_class parport_pc video parport evdev backlight
CPU: 0 PID: 21615 Comm: rmmod Tainted: G     U          4.4.0-rc4-mgm-ovl+ #4
Hardware name: FUJITSU SIEMENS LIFEBOOK S6120/FJNB16C, BIOS Version 1.26  05/10/2004
 e31a3de0 e31a3de0 e31a3d9c c128d4bd e31a3dd0 c1045a0c c15e00c4 e31a3dfc
 0000546f c15dfad2 000003ce c12b3740 000003ce c12b3740 00000000 00000001
 f61fb8a0 e31a3de8 c1045a83 00000009 e31a3de0 c15e00c4 e31a3dfc e31a3e4c
Call Trace:
 [<c128d4bd>] dump_stack+0x16/0x19
 [<c1045a0c>] warn_slowpath_common+0x8c/0xd0
 [<c12b3740>] ? dma_debug_device_change+0x190/0x1f0
 [<c12b3740>] ? dma_debug_device_change+0x190/0x1f0
 [<c1045a83>] warn_slowpath_fmt+0x33/0x40
 [<c12b3740>] dma_debug_device_change+0x190/0x1f0
 [<c1065499>] notifier_call_chain+0x59/0x70
 [<c10655af>] __blocking_notifier_call_chain+0x3f/0x80
 [<c106560f>] blocking_notifier_call_chain+0x1f/0x30
 [<c134cfb3>] __device_release_driver+0xc3/0xf0
 [<c134d0d7>] driver_detach+0x97/0xa0
 [<c134c440>] bus_remove_driver+0x40/0x90
 [<c134db18>] driver_unregister+0x28/0x60
 [<c1079e8c>] ? trace_hardirqs_on_caller+0x12c/0x1d0
 [<c12c0618>] pci_unregister_driver+0x18/0x80
 [<f83e96e7>] drm_pci_exit+0x87/0xb0 [drm]
 [<f8b3be2d>] i915_exit+0x1b/0x1ee [i915]
 [<c10b999c>] SyS_delete_module+0x14c/0x210
 [<c1079e8c>] ? trace_hardirqs_on_caller+0x12c/0x1d0
 [<c115a9bd>] ? ____fput+0xd/0x10
 [<c1002014>] do_fast_syscall_32+0xa4/0x450
 [<c149f6fa>] sysenter_past_esp+0x3b/0x5d
---[ end trace c2ecbc77760f10a0 ]---
Mapped at:
 [<c12b3183>] debug_dma_alloc_coherent+0x33/0x90
 [<f83e989c>] drm_pci_alloc+0x18c/0x1e0 [drm]
 [<f8acd59f>] intel_init_ring_buffer+0x2af/0x490 [i915]
 [<f8acd8b0>] intel_init_render_ring_buffer+0x130/0x750 [i915]
 [<f8aaea4e>] i915_gem_init_rings+0x1e/0x110 [i915]

v2: s/BUG_ON/WARN_ON/ since dim doens't like the former anymore

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Fixes: 5c6c600 ("drm/i915: Remove DRI1 ring accessors and API")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 339701d7a9a5..d9e0b400294d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1899,6 +1899,17 @@ i915_dispatch_execbuffer(struct drm_i915_gem_request *req,
 	return 0;
 }
 
+static void cleanup_phys_status_page(struct intel_engine_cs *ring)
+{
+	struct drm_i915_private *dev_priv = to_i915(ring->dev);
+
+	if (!dev_priv->status_page_dmah)
+		return;
+
+	drm_pci_free(ring->dev, dev_priv->status_page_dmah);
+	ring->status_page.page_addr = NULL;
+}
+
 static void cleanup_status_page(struct intel_engine_cs *ring)
 {
 	struct drm_i915_gem_object *obj;
@@ -1915,9 +1926,9 @@ static void cleanup_status_page(struct intel_engine_cs *ring)
 
 static int init_status_page(struct intel_engine_cs *ring)
 {
-	struct drm_i915_gem_object *obj;
+	struct drm_i915_gem_object *obj = ring->status_page.obj;
 
-	if ((obj = ring->status_page.obj) == NULL) {
+	if (obj == NULL) {
 		unsigned flags;
 		int ret;
 
@@ -2162,7 +2173,7 @@ static int intel_init_ring_buffer(struct drm_device *dev,
 		if (ret)
 			goto error;
 	} else {
-		BUG_ON(ring->id != RCS);
+		WARN_ON(ring->id != RCS);
 		ret = init_phys_status_page(ring);
 		if (ret)
 			goto error;
@@ -2208,7 +2219,12 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
 	if (ring->cleanup)
 		ring->cleanup(ring);
 
-	cleanup_status_page(ring);
+	if (I915_NEED_GFX_HWS(ring->dev)) {
+		cleanup_status_page(ring);
+	} else {
+		WARN_ON(ring->id != RCS);
+		cleanup_phys_status_page(ring);
+	}
 
 	i915_cmd_parser_fini_ring(ring);
 	i915_gem_batch_pool_fini(&ring->batch_pool);
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 02/10] drm/i915: Cleanup phys status page too

[Daniel Vetter]

On Mon, Jan 11, 2016 at 08:48:32PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Restore the lost phys status page cleanup.
> 
> Fixes the following splat with DMA_API_DEBUG=y:

Oh, we should enable this in our CI. Can you please shoot a mail to Tomi?

> 
> WARNING: CPU: 0 PID: 21615 at ../lib/dma-debug.c:974 dma_debug_device_change+0x190/0x1f0()
> pci 0000:00:02.0: DMA-API: device driver has pending DMA allocations while released from device [count=1]
>                One of leaked entries details: [device address=0x0000000023163000] [size=4096 bytes] [mapped with DMA_BIDIRECTIONAL] [mapped as coherent]
> Modules linked in: i915(-) i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm sha256_generic hmac drbg ctr ccm sch_fq_codel binfmt_misc joydev mousedev arc4 ath5k iTCO_wdt mac80211 smsc_ircc2 ath snd_intel8x0m snd_intel8x0 snd_ac97_codec ac97_bus psmouse snd_pcm input_leds i2c_i801 pcspkr snd_timer cfg80211 snd soundcore i2c_core ehci_pci firewire_ohci ehci_hcd firewire_core lpc_ich 8139too rfkill crc_itu_t mfd_core mii usbcore rng_core intel_agp intel_gtt usb_common agpgart irda crc_ccitt fujitsu_laptop led_class parport_pc video parport evdev backlight
> CPU: 0 PID: 21615 Comm: rmmod Tainted: G     U          4.4.0-rc4-mgm-ovl+ #4
> Hardware name: FUJITSU SIEMENS LIFEBOOK S6120/FJNB16C, BIOS Version 1.26  05/10/2004
>  e31a3de0 e31a3de0 e31a3d9c c128d4bd e31a3dd0 c1045a0c c15e00c4 e31a3dfc
>  0000546f c15dfad2 000003ce c12b3740 000003ce c12b3740 00000000 00000001
>  f61fb8a0 e31a3de8 c1045a83 00000009 e31a3de0 c15e00c4 e31a3dfc e31a3e4c
> Call Trace:
>  [<c128d4bd>] dump_stack+0x16/0x19
>  [<c1045a0c>] warn_slowpath_common+0x8c/0xd0
>  [<c12b3740>] ? dma_debug_device_change+0x190/0x1f0
>  [<c12b3740>] ? dma_debug_device_change+0x190/0x1f0
>  [<c1045a83>] warn_slowpath_fmt+0x33/0x40
>  [<c12b3740>] dma_debug_device_change+0x190/0x1f0
>  [<c1065499>] notifier_call_chain+0x59/0x70
>  [<c10655af>] __blocking_notifier_call_chain+0x3f/0x80
>  [<c106560f>] blocking_notifier_call_chain+0x1f/0x30
>  [<c134cfb3>] __device_release_driver+0xc3/0xf0
>  [<c134d0d7>] driver_detach+0x97/0xa0
>  [<c134c440>] bus_remove_driver+0x40/0x90
>  [<c134db18>] driver_unregister+0x28/0x60
>  [<c1079e8c>] ? trace_hardirqs_on_caller+0x12c/0x1d0
>  [<c12c0618>] pci_unregister_driver+0x18/0x80
>  [<f83e96e7>] drm_pci_exit+0x87/0xb0 [drm]
>  [<f8b3be2d>] i915_exit+0x1b/0x1ee [i915]
>  [<c10b999c>] SyS_delete_module+0x14c/0x210
>  [<c1079e8c>] ? trace_hardirqs_on_caller+0x12c/0x1d0
>  [<c115a9bd>] ? ____fput+0xd/0x10
>  [<c1002014>] do_fast_syscall_32+0xa4/0x450
>  [<c149f6fa>] sysenter_past_esp+0x3b/0x5d
> ---[ end trace c2ecbc77760f10a0 ]---
> Mapped at:
>  [<c12b3183>] debug_dma_alloc_coherent+0x33/0x90
>  [<f83e989c>] drm_pci_alloc+0x18c/0x1e0 [drm]
>  [<f8acd59f>] intel_init_ring_buffer+0x2af/0x490 [i915]
>  [<f8acd8b0>] intel_init_render_ring_buffer+0x130/0x750 [i915]
>  [<f8aaea4e>] i915_gem_init_rings+0x1e/0x110 [i915]
> 
> v2: s/BUG_ON/WARN_ON/ since dim doens't like the former anymore
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Fixes: 5c6c600 ("drm/i915: Remove DRI1 ring accessors and API")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 339701d7a9a5..d9e0b400294d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1899,6 +1899,17 @@ i915_dispatch_execbuffer(struct drm_i915_gem_request *req,
>  	return 0;
>  }
>  
> +static void cleanup_phys_status_page(struct intel_engine_cs *ring)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(ring->dev);
> +
> +	if (!dev_priv->status_page_dmah)
> +		return;
> +
> +	drm_pci_free(ring->dev, dev_priv->status_page_dmah);
> +	ring->status_page.page_addr = NULL;
> +}
> +
>  static void cleanup_status_page(struct intel_engine_cs *ring)
>  {
>  	struct drm_i915_gem_object *obj;
> @@ -1915,9 +1926,9 @@ static void cleanup_status_page(struct intel_engine_cs *ring)
>  
>  static int init_status_page(struct intel_engine_cs *ring)
>  {
> -	struct drm_i915_gem_object *obj;
> +	struct drm_i915_gem_object *obj = ring->status_page.obj;
>  
> -	if ((obj = ring->status_page.obj) == NULL) {
> +	if (obj == NULL) {
>  		unsigned flags;
>  		int ret;
>  
> @@ -2162,7 +2173,7 @@ static int intel_init_ring_buffer(struct drm_device *dev,
>  		if (ret)
>  			goto error;
>  	} else {
> -		BUG_ON(ring->id != RCS);
> +		WARN_ON(ring->id != RCS);
>  		ret = init_phys_status_page(ring);
>  		if (ret)
>  			goto error;
> @@ -2208,7 +2219,12 @@ void intel_cleanup_ring_buffer(struct intel_engine_cs *ring)
>  	if (ring->cleanup)
>  		ring->cleanup(ring);
>  
> -	cleanup_status_page(ring);
> +	if (I915_NEED_GFX_HWS(ring->dev)) {
> +		cleanup_status_page(ring);
> +	} else {
> +		WARN_ON(ring->id != RCS);
> +		cleanup_phys_status_page(ring);
> +	}
>  
>  	i915_cmd_parser_fini_ring(ring);
>  	i915_gem_batch_pool_fini(&ring->batch_pool);
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 03/10] drm/i915: Write out crc frame counts in hex

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The crc code assumes that the printed frame counter value takes
exactly 8 characters. The counter is a 32bit value, so to fit
it into 8 characters we need to print it in hex, in decimal it
would take 10 characters.

This obviously needs a corresponding change in intel-gpu-tools.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 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 24318b79bcfc..96d6e5de0811 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3518,7 +3518,7 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
 		pipe_crc->tail = (pipe_crc->tail + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
 
 		bytes_read += snprintf(buf, PIPE_CRC_BUFFER_LEN,
-				       "%8u %8x %8x %8x %8x %8x\n",
+				       "%8x %8x %8x %8x %8x %8x\n",
 				       entry->frame, entry->crc[0],
 				       entry->crc[1], entry->crc[2],
 				       entry->crc[3], entry->crc[4]);
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/10] drm/i915: Write out crc frame counts in hex

[Daniel Vetter]

On Mon, Dec 14, 2015 at 06:23:42PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The crc code assumes that the printed frame counter value takes
> exactly 8 characters. The counter is a 32bit value, so to fit
> it into 8 characters we need to print it in hex, in decimal it
> would take 10 characters.
> 
> This obviously needs a corresponding change in intel-gpu-tools.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

ugh, and we hardcode the same %8u on the igt side. What about instead
changing igt to just parse for an %u, push that, then change the kernel to
dump a %u without length? With a few months in between.

At least some backwards compat must be there, otherwise bisecting will be
slightly painful.
-Daniel

> ---
>  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 24318b79bcfc..96d6e5de0811 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3518,7 +3518,7 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
>  		pipe_crc->tail = (pipe_crc->tail + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
>  
>  		bytes_read += snprintf(buf, PIPE_CRC_BUFFER_LEN,
> -				       "%8u %8x %8x %8x %8x %8x\n",
> +				       "%8x %8x %8x %8x %8x %8x\n",
>  				       entry->frame, entry->crc[0],
>  				       entry->crc[1], entry->crc[2],
>  				       entry->crc[3], entry->crc[4]);
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/10] drm/i915: Write out crc frame counts in hex

[Ville Syrjälä]

On Wed, Dec 16, 2015 at 11:23:54AM +0100, Daniel Vetter wrote:
> On Mon, Dec 14, 2015 at 06:23:42PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The crc code assumes that the printed frame counter value takes
> > exactly 8 characters. The counter is a 32bit value, so to fit
> > it into 8 characters we need to print it in hex, in decimal it
> > would take 10 characters.
> > 
> > This obviously needs a corresponding change in intel-gpu-tools.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> ugh, and we hardcode the same %8u on the igt side. What about instead
> changing igt to just parse for an %u, push that, then change the kernel to
> dump a %u without length? With a few months in between.
> 
> At least some backwards compat must be there, otherwise bisecting will be
> slightly painful.

So the other idea I had was to limit the counter to 24 bits always. That
would avoid having to change the printf format string at all, and it
would gives us a consistent wraparound behaviour (since gen3/4 frame
counter is only 24 bits).


> -Daniel
> 
> > ---
> >  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 24318b79bcfc..96d6e5de0811 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -3518,7 +3518,7 @@ i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
> >  		pipe_crc->tail = (pipe_crc->tail + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
> >  
> >  		bytes_read += snprintf(buf, PIPE_CRC_BUFFER_LEN,
> > -				       "%8u %8x %8x %8x %8x %8x\n",
> > +				       "%8x %8x %8x %8x %8x %8x\n",
> >  				       entry->frame, entry->crc[0],
> >  				       entry->crc[1], entry->crc[2],
> >  				       entry->crc[3], entry->crc[4]);
> > -- 
> > 2.4.10
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/10] drm/i915: Write out crc frame counts in hex

[Daniel Vetter]

On Wed, Dec 16, 2015 at 12:58:33PM +0200, Ville Syrjälä wrote:
> On Wed, Dec 16, 2015 at 11:23:54AM +0100, Daniel Vetter wrote:
> > On Mon, Dec 14, 2015 at 06:23:42PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > The crc code assumes that the printed frame counter value takes
> > > exactly 8 characters. The counter is a 32bit value, so to fit
> > > it into 8 characters we need to print it in hex, in decimal it
> > > would take 10 characters.
> > > 
> > > This obviously needs a corresponding change in intel-gpu-tools.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > ugh, and we hardcode the same %8u on the igt side. What about instead
> > changing igt to just parse for an %u, push that, then change the kernel to
> > dump a %u without length? With a few months in between.
> > 
> > At least some backwards compat must be there, otherwise bisecting will be
> > slightly painful.
> 
> So the other idea I had was to limit the counter to 24 bits always. That
> would avoid having to change the printf format string at all, and it
> would gives us a consistent wraparound behaviour (since gen3/4 frame
> counter is only 24 bits).

Still a mess imo. I thik the real problem is that userspace doesn't know
what the wraparound max is, so impossible to handle that. I guess we could
spec that its 24 (or 20) bits or whatever, but that doesn't feel like a
great idea long-term. Especially since some folks have ideas about making
crc capture cross-vendor. If we need to redo this, might be better to dump
max_vblank_count too.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 04/10] drm/i915: Wait for pipe to start before sampling vblank timestamps on gen2

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We use the vblank timestamps to generate the vblank frame counter value
on gen2. That means we need the pipe scanout position to be accurate
when we call drm_crtc_vblank_on(), otherwise the frame counter
guesstimate may jump when the pipe actually start.

What I observed on my 85x is that the DSL initially reads 0, and when
the pipe actually starts DSL jumps to vblank_start. On gen2 DSL==0 means
actually vtotal-1 (see update_scanline_offset()), so if we initially
get vtotal-1, and then very quickly vblank_start (or thereabouts), the
scanout position will appear to jump backwards by approximately one
vblank length. Which means the frame counter guesstimate will also
jump backwards. That's no good, so let's make sure the pipe has
started before we call drm_crtc_vblank_on().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7dd7200d3ba9..78845d1cfd2e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2152,6 +2152,17 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 
 	I915_WRITE(reg, val | PIPECONF_ENABLE);
 	POSTING_READ(reg);
+
+	/*
+	 * Until the pipe starts DSL will read as 0, which would cause
+	 * an apparent vblank timestamp jump, which messes up also the
+	 * frame count when it's derived from the timestamps. So let's
+	 * wait for the pipe to start properly before we call
+	 * drm_crtc_vblank_on()
+	 */
+	if (dev->max_vblank_count == 0 &&
+	    wait_for(intel_get_crtc_scanline(crtc) != crtc->scanline_offset, 50))
+		DRM_ERROR("pipe %c didn't start\n", pipe_name(pipe));
 }
 
 /**
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/10] drm/i915: Wait for pipe to start before sampling vblank timestamps on gen2

[Daniel Vetter]

On Mon, Dec 14, 2015 at 06:23:43PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We use the vblank timestamps to generate the vblank frame counter value
> on gen2. That means we need the pipe scanout position to be accurate
> when we call drm_crtc_vblank_on(), otherwise the frame counter
> guesstimate may jump when the pipe actually start.
> 
> What I observed on my 85x is that the DSL initially reads 0, and when
> the pipe actually starts DSL jumps to vblank_start. On gen2 DSL==0 means
> actually vtotal-1 (see update_scanline_offset()), so if we initially
> get vtotal-1, and then very quickly vblank_start (or thereabouts), the
> scanout position will appear to jump backwards by approximately one
> vblank length. Which means the frame counter guesstimate will also
> jump backwards. That's no good, so let's make sure the pipe has
> started before we call drm_crtc_vblank_on().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Can't confirm your hw findings, but makes sense. And not the first bug
we've fixed with a "let's just wait for a bit" in the vblank area.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7dd7200d3ba9..78845d1cfd2e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2152,6 +2152,17 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
>  
>  	I915_WRITE(reg, val | PIPECONF_ENABLE);
>  	POSTING_READ(reg);
> +
> +	/*
> +	 * Until the pipe starts DSL will read as 0, which would cause
> +	 * an apparent vblank timestamp jump, which messes up also the
> +	 * frame count when it's derived from the timestamps. So let's
> +	 * wait for the pipe to start properly before we call
> +	 * drm_crtc_vblank_on()
> +	 */
> +	if (dev->max_vblank_count == 0 &&
> +	    wait_for(intel_get_crtc_scanline(crtc) != crtc->scanline_offset, 50))
> +		DRM_ERROR("pipe %c didn't start\n", pipe_name(pipe));
>  }
>  
>  /**
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/10] drm/i915: Use drm_vblank_count() on gen2 for crc frame count

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Gen2 doesn't have a hardware frame counter, so let's use the sw
counter value instead.

Testcase: igt/kms_pipe_crc_basic/read-crc-pipe-?-frame-sequence
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 11 +++++++++++
 drivers/gpu/drm/i915/i915_irq.c     |  5 ++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 96d6e5de0811..695c69e85374 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4021,6 +4021,14 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
 		if (!entries)
 			return -ENOMEM;
 
+		if (dev->max_vblank_count == 0) {
+			ret = drm_vblank_get(dev, pipe);
+			if (ret) {
+				kfree(entries);
+				return ret;
+			}
+		}
+
 		/*
 		 * When IPS gets enabled, the pipe CRC changes. Since IPS gets
 		 * enabled and disabled dynamically based on package C states,
@@ -4073,6 +4081,9 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
 			hsw_trans_edp_pipe_A_crc_wa(dev, false);
 
 		hsw_enable_ips(crtc);
+
+		if (dev->max_vblank_count == 0)
+			drm_vblank_put(dev, pipe);
 	}
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 86664d1b3389..37ec8427359a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1533,7 +1533,10 @@ static void display_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe,
 
 	entry = &pipe_crc->entries[head];
 
-	entry->frame = dev->driver->get_vblank_counter(dev, pipe);
+	if (dev->max_vblank_count == 0)
+		entry->frame = drm_vblank_count(dev, pipe);
+	else
+		entry->frame = dev->driver->get_vblank_counter(dev, pipe);
 	entry->crc[0] = crc0;
 	entry->crc[1] = crc1;
 	entry->crc[2] = crc2;
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/10] drm/i915: Use drm_vblank_count() on gen2 for crc frame count

[Daniel Vetter]

On Mon, Dec 14, 2015 at 06:23:44PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Gen2 doesn't have a hardware frame counter, so let's use the sw
> counter value instead.
> 
> Testcase: igt/kms_pipe_crc_basic/read-crc-pipe-?-frame-sequence
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

I think the better test is skip the testcase if all frame numbers are 0.
Not sure it's worth it to hack this up.

But if you disagree I'd just throw out all the max_vblank_count checks and
unconditionally enable the vblank counter and just always use
drm_vblank_count. I don't think this can hurt use while we use the CRC,
since it shouldn't generate more interrupts.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 11 +++++++++++
>  drivers/gpu/drm/i915/i915_irq.c     |  5 ++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 96d6e5de0811..695c69e85374 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4021,6 +4021,14 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
>  		if (!entries)
>  			return -ENOMEM;
>  
> +		if (dev->max_vblank_count == 0) {
> +			ret = drm_vblank_get(dev, pipe);
> +			if (ret) {
> +				kfree(entries);
> +				return ret;
> +			}
> +		}
> +
>  		/*
>  		 * When IPS gets enabled, the pipe CRC changes. Since IPS gets
>  		 * enabled and disabled dynamically based on package C states,
> @@ -4073,6 +4081,9 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
>  			hsw_trans_edp_pipe_A_crc_wa(dev, false);
>  
>  		hsw_enable_ips(crtc);
> +
> +		if (dev->max_vblank_count == 0)
> +			drm_vblank_put(dev, pipe);
>  	}
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 86664d1b3389..37ec8427359a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1533,7 +1533,10 @@ static void display_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe,
>  
>  	entry = &pipe_crc->entries[head];
>  
> -	entry->frame = dev->driver->get_vblank_counter(dev, pipe);
> +	if (dev->max_vblank_count == 0)
> +		entry->frame = drm_vblank_count(dev, pipe);
> +	else
> +		entry->frame = dev->driver->get_vblank_counter(dev, pipe);
>  	entry->crc[0] = crc0;
>  	entry->crc[1] = crc1;
>  	entry->crc[2] = crc2;
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/10] drm/i915: Use drm_vblank_count() on gen2 for crc frame count

[Ville Syrjälä]

On Wed, Dec 16, 2015 at 11:30:19AM +0100, Daniel Vetter wrote:
> On Mon, Dec 14, 2015 at 06:23:44PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Gen2 doesn't have a hardware frame counter, so let's use the sw
> > counter value instead.
> > 
> > Testcase: igt/kms_pipe_crc_basic/read-crc-pipe-?-frame-sequence
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I think the better test is skip the testcase if all frame numbers are 0.
> Not sure it's worth it to hack this up.

What's the problem with it? A few extra lines of code?

> 
> But if you disagree I'd just throw out all the max_vblank_count checks and
> unconditionally enable the vblank counter and just always use
> drm_vblank_count. I don't think this can hurt use while we use the CRC,
> since it shouldn't generate more interrupts.

I'd rather not do that since the vblank interrupts might hide some bugs
(eg. missing crc done interrupts). Also I'm not convinced the software
counter isn't racy wrt. the crc done interrupt. Would need to check on
every platform to make sure the vblank interrupt is raised and processed
before the crc done interrupt, or we'd need to switch over to using
the vblank interrupt for crc gathering.

Note that I didn't actually check which order the interrupts fire on
gen2, but this was enough to make the current tests pass at least.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 11 +++++++++++
> >  drivers/gpu/drm/i915/i915_irq.c     |  5 ++++-
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 96d6e5de0811..695c69e85374 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -4021,6 +4021,14 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
> >  		if (!entries)
> >  			return -ENOMEM;
> >  
> > +		if (dev->max_vblank_count == 0) {
> > +			ret = drm_vblank_get(dev, pipe);
> > +			if (ret) {
> > +				kfree(entries);
> > +				return ret;
> > +			}
> > +		}
> > +
> >  		/*
> >  		 * When IPS gets enabled, the pipe CRC changes. Since IPS gets
> >  		 * enabled and disabled dynamically based on package C states,
> > @@ -4073,6 +4081,9 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe,
> >  			hsw_trans_edp_pipe_A_crc_wa(dev, false);
> >  
> >  		hsw_enable_ips(crtc);
> > +
> > +		if (dev->max_vblank_count == 0)
> > +			drm_vblank_put(dev, pipe);
> >  	}
> >  
> >  	return 0;
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 86664d1b3389..37ec8427359a 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1533,7 +1533,10 @@ static void display_pipe_crc_irq_handler(struct drm_device *dev, enum pipe pipe,
> >  
> >  	entry = &pipe_crc->entries[head];
> >  
> > -	entry->frame = dev->driver->get_vblank_counter(dev, pipe);
> > +	if (dev->max_vblank_count == 0)
> > +		entry->frame = drm_vblank_count(dev, pipe);
> > +	else
> > +		entry->frame = dev->driver->get_vblank_counter(dev, pipe);
> >  	entry->crc[0] = crc0;
> >  	entry->crc[1] = crc1;
> >  	entry->crc[2] = crc2;
> > -- 
> > 2.4.10
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/10] drm/i915: Use drm_vblank_count() on gen2 for crc frame count

[Daniel Vetter]

On Wed, Dec 16, 2015 at 02:51:20PM +0200, Ville Syrjälä wrote:
> On Wed, Dec 16, 2015 at 11:30:19AM +0100, Daniel Vetter wrote:
> > On Mon, Dec 14, 2015 at 06:23:44PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Gen2 doesn't have a hardware frame counter, so let's use the sw
> > > counter value instead.
> > > 
> > > Testcase: igt/kms_pipe_crc_basic/read-crc-pipe-?-frame-sequence
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > I think the better test is skip the testcase if all frame numbers are 0.
> > Not sure it's worth it to hack this up.
> 
> What's the problem with it? A few extra lines of code?

Well the idea of sampling the hw irq counter is that we'd notice when we
start missing something. Sampling the baked vblank counter isn't really
useful in that regard I think.

Otoh we could just sample the official vblank counter, and then a test
could schedule a flip for a given frame and check that the crc changes at
the right frame (using continuous sampling). That would indeed be useful.
This here just looks like a few random changes to appease a fairly
arbitrary testcase.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/10] drm/i915: Use drm_vblank_count() on gen2 for crc frame count

[Ville Syrjälä]

On Wed, Dec 16, 2015 at 04:28:36PM +0100, Daniel Vetter wrote:
> On Wed, Dec 16, 2015 at 02:51:20PM +0200, Ville Syrjälä wrote:
> > On Wed, Dec 16, 2015 at 11:30:19AM +0100, Daniel Vetter wrote:
> > > On Mon, Dec 14, 2015 at 06:23:44PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Gen2 doesn't have a hardware frame counter, so let's use the sw
> > > > counter value instead.
> > > > 
> > > > Testcase: igt/kms_pipe_crc_basic/read-crc-pipe-?-frame-sequence
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > I think the better test is skip the testcase if all frame numbers are 0.
> > > Not sure it's worth it to hack this up.
> > 
> > What's the problem with it? A few extra lines of code?
> 
> Well the idea of sampling the hw irq counter is that we'd notice when we
> start missing something. Sampling the baked vblank counter isn't really
> useful in that regard I think.

I think only if we'd also miss the vblank interrupt somehow.

> 
> Otoh we could just sample the official vblank counter, and then a test
> could schedule a flip for a given frame and check that the crc changes at
> the right frame (using continuous sampling). That would indeed be useful.

That's a decent point. Maybe we would want to expose both counters
actually and nag if they don't match? That sort of thing could be useful
just to validate the vblank code too, but we don't have any interface
to expose the hw counter alongside the sw counter for that purpose.

> This here just looks like a few random changes to appease a fairly
> arbitrary testcase.
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/10] drm/i915: Use drm_vblank_count() on gen2 for crc frame count

[Daniel Vetter]

On Wed, Dec 16, 2015 at 07:22:25PM +0200, Ville Syrjälä wrote:
> On Wed, Dec 16, 2015 at 04:28:36PM +0100, Daniel Vetter wrote:
> > On Wed, Dec 16, 2015 at 02:51:20PM +0200, Ville Syrjälä wrote:
> > > On Wed, Dec 16, 2015 at 11:30:19AM +0100, Daniel Vetter wrote:
> > > > On Mon, Dec 14, 2015 at 06:23:44PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > Gen2 doesn't have a hardware frame counter, so let's use the sw
> > > > > counter value instead.
> > > > > 
> > > > > Testcase: igt/kms_pipe_crc_basic/read-crc-pipe-?-frame-sequence
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > I think the better test is skip the testcase if all frame numbers are 0.
> > > > Not sure it's worth it to hack this up.
> > > 
> > > What's the problem with it? A few extra lines of code?
> > 
> > Well the idea of sampling the hw irq counter is that we'd notice when we
> > start missing something. Sampling the baked vblank counter isn't really
> > useful in that regard I think.
> 
> I think only if we'd also miss the vblank interrupt somehow.
> 
> > 
> > Otoh we could just sample the official vblank counter, and then a test
> > could schedule a flip for a given frame and check that the crc changes at
> > the right frame (using continuous sampling). That would indeed be useful.
> 
> That's a decent point. Maybe we would want to expose both counters
> actually and nag if they don't match? That sort of thing could be useful
> just to validate the vblank code too, but we don't have any interface
> to expose the hw counter alongside the sw counter for that purpose.

Hm yeah that makes sense. Makes the backward compat dance in igt a bit
more annoying though. But should be doable by first trying to parse the
new layout and then falling back to the old one if it fails.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 06/10] drm/i915: Enable vblank_disable_immediate on gen2

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Since
commit 4dfd648 ("drm: Use vblank timestamps to guesstimate how many vblanks were missed")
the vblank code can cook up a frame counter value based on
the vblank timestamps (as long as they're accurate), so there's
no longer any need to keep vblank interrupts enabled on gen2
when no one is interested in them. So let's opt into the
immediate disable scheme on gen2 as well.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 37ec8427359a..111edda1e73d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4437,14 +4437,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 		dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
 	}
 
-	/*
-	 * Opt out of the vblank disable timer on everything except gen2.
-	 * Gen2 doesn't have a hardware frame counter and so depends on
-	 * vblank interrupts to produce sane vblank seuquence numbers.
-	 */
-	if (!IS_GEN2(dev_priv))
-		dev->vblank_disable_immediate = true;
-
+	dev->vblank_disable_immediate = true;
 	dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp;
 	dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
 
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/10] drm/i915: Enable vblank_disable_immediate on gen2

[Daniel Vetter]

On Mon, Dec 14, 2015 at 06:23:45PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since
> commit 4dfd648 ("drm: Use vblank timestamps to guesstimate how many vblanks were missed")
> the vblank code can cook up a frame counter value based on
> the vblank timestamps (as long as they're accurate), so there's
> no longer any need to keep vblank interrupts enabled on gen2
> when no one is interested in them. So let's opt into the
> immediate disable scheme on gen2 as well.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Yeah makes sense. Userspace that cares can simply ask for a vblank event
in 1000 frames or so, to keep the vblank interrupt enabled.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/i915_irq.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 37ec8427359a..111edda1e73d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4437,14 +4437,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
>  		dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
>  	}
>  
> -	/*
> -	 * Opt out of the vblank disable timer on everything except gen2.
> -	 * Gen2 doesn't have a hardware frame counter and so depends on
> -	 * vblank interrupts to produce sane vblank seuquence numbers.
> -	 */
> -	if (!IS_GEN2(dev_priv))
> -		dev->vblank_disable_immediate = true;
> -
> +	dev->vblank_disable_immediate = true;
>  	dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp;
>  	dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
>  
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/10] drm/i915: Enable vblank_disable_immediate on gen2

[Chris Wilson]

On Wed, Dec 16, 2015 at 11:31:31AM +0100, Daniel Vetter wrote:
> On Mon, Dec 14, 2015 at 06:23:45PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Since
> > commit 4dfd648 ("drm: Use vblank timestamps to guesstimate how many vblanks were missed")
> > the vblank code can cook up a frame counter value based on
> > the vblank timestamps (as long as they're accurate), so there's
> > no longer any need to keep vblank interrupts enabled on gen2
> > when no one is interested in them. So let's opt into the
> > immediate disable scheme on gen2 as well.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Yeah makes sense. Userspace that cares can simply ask for a vblank event
> in 1000 frames or so, to keep the vblank interrupt enabled.

I prefer my don't do immediate_disable until after we finish the vblank
event.

I have vblank/flip keepalives in userspace - but at the end of the day
they just add work when we can make a change in the kernel to reduce work.
-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

[PATCH 07/10] drm/i915: Allow 27 bytes child_dev for VBT <109

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

My 85x has VBT version 108 which has a child dev size of 27 bytes.
Let's allow that without printing an error.

We still want to reject the actual parsin since for that we need
the child device size to be at least 33 bytes. So we should still
check for that, but let's make it print a debug message only instead
of an error.

While at it, toss in a BUILD_BUG_ON() to verify our struct
old_child_dev_config is in fact 33 bytes.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 070470fe9a91..770b825dabc0 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1089,7 +1089,10 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 		DRM_DEBUG_KMS("No general definition block is found, no devices defined.\n");
 		return;
 	}
-	if (bdb->version < 195) {
+	if (bdb->version < 109) {
+		expected_size = 27;
+	} else if (bdb->version < 195) {
+		BUILD_BUG_ON(sizeof(struct old_child_dev_config) != 33);
 		expected_size = sizeof(struct old_child_dev_config);
 	} else if (bdb->version == 195) {
 		expected_size = 37;
@@ -1102,18 +1105,18 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 				 bdb->version, expected_size);
 	}
 
-	/* The legacy sized child device config is the minimum we need. */
-	if (p_defs->child_dev_size < sizeof(struct old_child_dev_config)) {
-		DRM_ERROR("Child device config size %u is too small.\n",
-			  p_defs->child_dev_size);
-		return;
-	}
-
 	/* Flag an error for unexpected size, but continue anyway. */
 	if (p_defs->child_dev_size != expected_size)
 		DRM_ERROR("Unexpected child device config size %u (expected %u for VBT version %u)\n",
 			  p_defs->child_dev_size, expected_size, bdb->version);
 
+	/* The legacy sized child device config is the minimum we need. */
+	if (p_defs->child_dev_size < sizeof(struct old_child_dev_config)) {
+		DRM_DEBUG_KMS("Child device config size %u is too small.\n",
+			      p_defs->child_dev_size);
+		return;
+	}
+
 	/* get the block size of general definitions */
 	block_size = get_blocksize(p_defs);
 	/* get the number of child device */
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 07/10] drm/i915: Allow 27 bytes child_dev for VBT <109

[Jani Nikula]

On Mon, 14 Dec 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> My 85x has VBT version 108 which has a child dev size of 27 bytes.
> Let's allow that without printing an error.
>
> We still want to reject the actual parsin since for that we need
> the child device size to be at least 33 bytes. So we should still
> check for that, but let's make it print a debug message only instead
> of an error.
>
> While at it, toss in a BUILD_BUG_ON() to verify our struct
> old_child_dev_config is in fact 33 bytes.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

This is scary stuff, but

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

as far as the code goes. I don't have the history books to check this
against.

> ---
>  drivers/gpu/drm/i915/intel_bios.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 070470fe9a91..770b825dabc0 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1089,7 +1089,10 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>  		DRM_DEBUG_KMS("No general definition block is found, no devices defined.\n");
>  		return;
>  	}
> -	if (bdb->version < 195) {
> +	if (bdb->version < 109) {
> +		expected_size = 27;
> +	} else if (bdb->version < 195) {
> +		BUILD_BUG_ON(sizeof(struct old_child_dev_config) != 33);
>  		expected_size = sizeof(struct old_child_dev_config);
>  	} else if (bdb->version == 195) {
>  		expected_size = 37;
> @@ -1102,18 +1105,18 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>  				 bdb->version, expected_size);
>  	}
>  
> -	/* The legacy sized child device config is the minimum we need. */
> -	if (p_defs->child_dev_size < sizeof(struct old_child_dev_config)) {
> -		DRM_ERROR("Child device config size %u is too small.\n",
> -			  p_defs->child_dev_size);
> -		return;
> -	}
> -
>  	/* Flag an error for unexpected size, but continue anyway. */
>  	if (p_defs->child_dev_size != expected_size)
>  		DRM_ERROR("Unexpected child device config size %u (expected %u for VBT version %u)\n",
>  			  p_defs->child_dev_size, expected_size, bdb->version);
>  
> +	/* The legacy sized child device config is the minimum we need. */
> +	if (p_defs->child_dev_size < sizeof(struct old_child_dev_config)) {
> +		DRM_DEBUG_KMS("Child device config size %u is too small.\n",
> +			      p_defs->child_dev_size);
> +		return;
> +	}
> +
>  	/* get the block size of general definitions */
>  	block_size = get_blocksize(p_defs);
>  	/* get the number of child device */

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 08/10] drm/i915: Expect child dev size of 22 bytes for VBT < 106

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

My 830 has VBT version 105 with child device size of 22 bytes.
Let's assume that's correct and adjust our expectations.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_bios.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index 770b825dabc0..c99a96989113 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -1089,7 +1089,9 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
 		DRM_DEBUG_KMS("No general definition block is found, no devices defined.\n");
 		return;
 	}
-	if (bdb->version < 109) {
+	if (bdb->version < 106) {
+		expected_size = 22;
+	} else if (bdb->version < 109) {
 		expected_size = 27;
 	} else if (bdb->version < 195) {
 		BUILD_BUG_ON(sizeof(struct old_child_dev_config) != 33);
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 08/10] drm/i915: Expect child dev size of 22 bytes for VBT < 106

[Jani Nikula]

On Mon, 14 Dec 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> My 830 has VBT version 105 with child device size of 22 bytes.
> Let's assume that's correct and adjust our expectations.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Let's see what blows up...

Acked-by: Jani Nikula <jani.nikula@intel.com>



> ---
>  drivers/gpu/drm/i915/intel_bios.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index 770b825dabc0..c99a96989113 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1089,7 +1089,9 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
>  		DRM_DEBUG_KMS("No general definition block is found, no devices defined.\n");
>  		return;
>  	}
> -	if (bdb->version < 109) {
> +	if (bdb->version < 106) {
> +		expected_size = 22;
> +	} else if (bdb->version < 109) {
>  		expected_size = 27;
>  	} else if (bdb->version < 195) {
>  		BUILD_BUG_ON(sizeof(struct old_child_dev_config) != 33);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/10] drm/i915: Reject < 8 byte batches on 830/845

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We use MI_BATCH_BUFFER on 830/845, which means we need to know
the batch length. If the user passes a bad batch length (< 8)
the results is most likely a GPU hang. Rejct such batches.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 0c005a3dc57f..e878375be7eb 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1825,6 +1825,9 @@ i830_dispatch_execbuffer(struct drm_i915_gem_request *req,
 	u32 cs_offset = ring->scratch.gtt_offset;
 	int ret;
 
+	if (len < 8)
+		return -EINVAL;
+
 	ret = intel_ring_begin(req, 6);
 	if (ret)
 		return ret;
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/10] drm/i915: Reject < 8 byte batches on 830/845

[Chris Wilson]

On Mon, Dec 14, 2015 at 06:23:48PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We use MI_BATCH_BUFFER on 830/845, which means we need to know
> the batch length. If the user passes a bad batch length (< 8)
> the results is most likely a GPU hang. Rejct such batches.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Though, do you then remove the discrepancy in the next MI_BB_START
patch?
-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: [PATCH 09/10] drm/i915: Reject < 8 byte batches on 830/845

[Ville Syrjälä]

On Mon, Dec 14, 2015 at 05:07:50PM +0000, Chris Wilson wrote:
> On Mon, Dec 14, 2015 at 06:23:48PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We use MI_BATCH_BUFFER on 830/845, which means we need to know
> > the batch length. If the user passes a bad batch length (< 8)
> > the results is most likely a GPU hang. Rejct such batches.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Though, do you then remove the discrepancy in the next MI_BB_START
> patch?

Yeah, I figured I'd still keep this in case the user passes in
bogus batch_len, but now that I think about it, there's no way for
values in the 1-7 range to reach us here due to the earlier 
'batch_len & 7' check, and 0 would get replaced by the obj size.
I guess we could drop this patch then.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/10] drm/i915: Reject < 8 byte batches on 830/845

[Daniel Vetter]

On Mon, Dec 14, 2015 at 06:23:48PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We use MI_BATCH_BUFFER on 830/845, which means we need to know
> the batch length. If the user passes a bad batch length (< 8)
> the results is most likely a GPU hang. Rejct such batches.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Can't we just reject a 0 batch_len everywhere? Well fix up igt first ofc.
Slipping a 0 through really shouldn't be allowed.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 0c005a3dc57f..e878375be7eb 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1825,6 +1825,9 @@ i830_dispatch_execbuffer(struct drm_i915_gem_request *req,
>  	u32 cs_offset = ring->scratch.gtt_offset;
>  	int ret;
>  
> +	if (len < 8)
> +		return -EINVAL;
> +
>  	ret = intel_ring_begin(req, 6);
>  	if (ret)
>  		return ret;
> -- 
> 2.4.10
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 09/10] drm/i915: Reject < 8 byte batches on 830/845

[Chris Wilson]

On Wed, Dec 16, 2015 at 11:36:31AM +0100, Daniel Vetter wrote:
> On Mon, Dec 14, 2015 at 06:23:48PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We use MI_BATCH_BUFFER on 830/845, which means we need to know
> > the batch length. If the user passes a bad batch length (< 8)
> > the results is most likely a GPU hang. Rejct such batches.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Can't we just reject a 0 batch_len everywhere? Well fix up igt first ofc.
> Slipping a 0 through really shouldn't be allowed.

No, I sneaked that bit of ABI abuse passed you. Sorry.
-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: [PATCH 09/10] drm/i915: Reject < 8 byte batches on 830/845

[Daniel Vetter]

On Wed, Dec 16, 2015 at 10:43:54AM +0000, Chris Wilson wrote:
> On Wed, Dec 16, 2015 at 11:36:31AM +0100, Daniel Vetter wrote:
> > On Mon, Dec 14, 2015 at 06:23:48PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > We use MI_BATCH_BUFFER on 830/845, which means we need to know
> > > the batch length. If the user passes a bad batch length (< 8)
> > > the results is most likely a GPU hang. Rejct such batches.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Can't we just reject a 0 batch_len everywhere? Well fix up igt first ofc.
> > Slipping a 0 through really shouldn't be allowed.
> 
> No, I sneaked that bit of ABI abuse passed you. Sorry.

Hm, still should augment gem_exec_params to at least attempt ugly stuff
like batch_len = 4 or batch_len = 3.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/10] drm/i915: Use MI_BATCH_BUFFER_START on 830/845

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

MI_BATCH_BUFFER is nasty since it requires that userspace pass in the
correct batch length.

Let's switch to using MI_BATCH_BUFFER_START instead (like we do on
other platforms). Then we don't have to specify the batch length
at all, and the CS will instead execute until it sees the
MI_BATCH_BUFFER_END.

We still need the batch length since we do the CS TLB workaround
and copy the batch into the permanently pinned scratch object
and execute it from there. But for this we can simply use the
batch object length when the user hasn't specified the actual
batch length. So specifying the batch length becomes just a
way to optimize the batch copy a little bit.

We lost batch_len from a bunch of igts (including the quiesce batch)
so without this igt is utterly broken on 830/845. Also some igts such
as gem_cpu_reloc never specified the batch_len and so didn't work.
With MI_BATCH_BUFFER_START we don't have to fix up igt every time
someone forgets that 830/845 exist.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 +++
 drivers/gpu/drm/i915/intel_ringbuffer.c    | 6 ++----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 5d01ea680dc1..be9fb08fccff 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1269,6 +1269,9 @@ i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params,
 	exec_start = params->batch_obj_vm_offset +
 		     params->args_batch_start_offset;
 
+	if (exec_len == 0)
+		exec_len = params->batch_obj->base.size;
+
 	ret = ring->dispatch_execbuffer(params->request,
 					exec_start, exec_len,
 					params->dispatch_flags);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index e878375be7eb..71569bc3d907 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1868,15 +1868,13 @@ i830_dispatch_execbuffer(struct drm_i915_gem_request *req,
 		offset = cs_offset;
 	}
 
-	ret = intel_ring_begin(req, 4);
+	ret = intel_ring_begin(req, 2);
 	if (ret)
 		return ret;
 
-	intel_ring_emit(ring, MI_BATCH_BUFFER);
+	intel_ring_emit(ring, MI_BATCH_BUFFER_START | MI_BATCH_GTT);
 	intel_ring_emit(ring, offset | (dispatch_flags & I915_DISPATCH_SECURE ?
 					0 : MI_BATCH_NON_SECURE));
-	intel_ring_emit(ring, offset + len - 8);
-	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
 
 	return 0;
-- 
2.4.10

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/10] drm/i915: Use MI_BATCH_BUFFER_START on 830/845

[Chris Wilson]

On Mon, Dec 14, 2015 at 06:23:49PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> MI_BATCH_BUFFER is nasty since it requires that userspace pass in the
> correct batch length.
> 
> Let's switch to using MI_BATCH_BUFFER_START instead (like we do on
> other platforms). Then we don't have to specify the batch length
> at all, and the CS will instead execute until it sees the
> MI_BATCH_BUFFER_END.

Oh. My. Gosh. There's a BB_START?!!!

> We still need the batch length since we do the CS TLB workaround
> and copy the batch into the permanently pinned scratch object
> and execute it from there. But for this we can simply use the
> batch object length when the user hasn't specified the actual
> batch length. So specifying the batch length becomes just a
> way to optimize the batch copy a little bit.
> 
> We lost batch_len from a bunch of igts (including the quiesce batch)
> so without this igt is utterly broken on 830/845. Also some igts such
> as gem_cpu_reloc never specified the batch_len and so didn't work.
> With MI_BATCH_BUFFER_START we don't have to fix up igt every time
> someone forgets that 830/845 exist.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Looks sane.
-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: [PATCH 10/10] drm/i915: Use MI_BATCH_BUFFER_START on 830/845

[Ville Syrjälä]

On Mon, Dec 14, 2015 at 04:58:54PM +0000, Chris Wilson wrote:
> On Mon, Dec 14, 2015 at 06:23:49PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > MI_BATCH_BUFFER is nasty since it requires that userspace pass in the
> > correct batch length.
> > 
> > Let's switch to using MI_BATCH_BUFFER_START instead (like we do on
> > other platforms). Then we don't have to specify the batch length
> > at all, and the CS will instead execute until it sees the
> > MI_BATCH_BUFFER_END.
> 
> Oh. My. Gosh. There's a BB_START?!!!

Looks like ;) At least my 830 seems perfectly happy with it. Well,
as happy as it's ever been. Though I still couldn't get it complete
a piglit run without hanging last I tried.

> 
> > We still need the batch length since we do the CS TLB workaround
> > and copy the batch into the permanently pinned scratch object
> > and execute it from there. But for this we can simply use the
> > batch object length when the user hasn't specified the actual
> > batch length. So specifying the batch length becomes just a
> > way to optimize the batch copy a little bit.
> > 
> > We lost batch_len from a bunch of igts (including the quiesce batch)
> > so without this igt is utterly broken on 830/845. Also some igts such
> > as gem_cpu_reloc never specified the batch_len and so didn't work.
> > With MI_BATCH_BUFFER_START we don't have to fix up igt every time
> > someone forgets that 830/845 exist.
> > 
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Looks sane.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/10] drm/i915: Use MI_BATCH_BUFFER_START on 830/845

[Chris Wilson]

On Mon, Dec 14, 2015 at 07:25:13PM +0200, Ville Syrjälä wrote:
> On Mon, Dec 14, 2015 at 04:58:54PM +0000, Chris Wilson wrote:
> > On Mon, Dec 14, 2015 at 06:23:49PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > MI_BATCH_BUFFER is nasty since it requires that userspace pass in the
> > > correct batch length.
> > > 
> > > Let's switch to using MI_BATCH_BUFFER_START instead (like we do on
> > > other platforms). Then we don't have to specify the batch length
> > > at all, and the CS will instead execute until it sees the
> > > MI_BATCH_BUFFER_END.
> > 
> > Oh. My. Gosh. There's a BB_START?!!!
> 
> Looks like ;) At least my 830 seems perfectly happy with it. Well,
> as happy as it's ever been. Though I still couldn't get it complete
> a piglit run without hanging last I tried.
> 
> > 
> > > We still need the batch length since we do the CS TLB workaround
> > > and copy the batch into the permanently pinned scratch object
> > > and execute it from there. But for this we can simply use the
> > > batch object length when the user hasn't specified the actual
> > > batch length. So specifying the batch length becomes just a
> > > way to optimize the batch copy a little bit.
> > > 
> > > We lost batch_len from a bunch of igts (including the quiesce batch)
> > > so without this igt is utterly broken on 830/845. Also some igts such
> > > as gem_cpu_reloc never specified the batch_len and so didn't work.
> > > With MI_BATCH_BUFFER_START we don't have to fix up igt every time
> > > someone forgets that 830/845 exist.
> > > 
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Looks sane.

Checked against bspec, and it does indeed list BB_START for 830/845 and
we already comply with the errata.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/10] drm/i915: Use MI_BATCH_BUFFER_START on 830/845

[Chris Wilson]

On Tue, Dec 15, 2015 at 10:09:30AM +0000, Chris Wilson wrote:
> On Mon, Dec 14, 2015 at 07:25:13PM +0200, Ville Syrjälä wrote:
> > On Mon, Dec 14, 2015 at 04:58:54PM +0000, Chris Wilson wrote:
> > > On Mon, Dec 14, 2015 at 06:23:49PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > MI_BATCH_BUFFER is nasty since it requires that userspace pass in the
> > > > correct batch length.
> > > > 
> > > > Let's switch to using MI_BATCH_BUFFER_START instead (like we do on
> > > > other platforms). Then we don't have to specify the batch length
> > > > at all, and the CS will instead execute until it sees the
> > > > MI_BATCH_BUFFER_END.
> > > 
> > > Oh. My. Gosh. There's a BB_START?!!!
> > 
> > Looks like ;) At least my 830 seems perfectly happy with it. Well,
> > as happy as it's ever been. Though I still couldn't get it complete
> > a piglit run without hanging last I tried.
> > 
> > > 
> > > > We still need the batch length since we do the CS TLB workaround
> > > > and copy the batch into the permanently pinned scratch object
> > > > and execute it from there. But for this we can simply use the
> > > > batch object length when the user hasn't specified the actual
> > > > batch length. So specifying the batch length becomes just a
> > > > way to optimize the batch copy a little bit.
> > > > 
> > > > We lost batch_len from a bunch of igts (including the quiesce batch)
> > > > so without this igt is utterly broken on 830/845. Also some igts such
> > > > as gem_cpu_reloc never specified the batch_len and so didn't work.
> > > > With MI_BATCH_BUFFER_START we don't have to fix up igt every time
> > > > someone forgets that 830/845 exist.
> > > > 
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Looks sane.
> 
> Checked against bspec, and it does indeed list BB_START for 830/845 and
> we already comply with the errata.

The other question, does this obsolete our work around? Can I be that
optimisitic?
-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: [PATCH 10/10] drm/i915: Use MI_BATCH_BUFFER_START on 830/845

[Ville Syrjälä]

On Tue, Dec 15, 2015 at 10:24:13AM +0000, Chris Wilson wrote:
> On Tue, Dec 15, 2015 at 10:09:30AM +0000, Chris Wilson wrote:
> > On Mon, Dec 14, 2015 at 07:25:13PM +0200, Ville Syrjälä wrote:
> > > On Mon, Dec 14, 2015 at 04:58:54PM +0000, Chris Wilson wrote:
> > > > On Mon, Dec 14, 2015 at 06:23:49PM +0200, ville.syrjala@linux.intel.com wrote:
> > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > 
> > > > > MI_BATCH_BUFFER is nasty since it requires that userspace pass in the
> > > > > correct batch length.
> > > > > 
> > > > > Let's switch to using MI_BATCH_BUFFER_START instead (like we do on
> > > > > other platforms). Then we don't have to specify the batch length
> > > > > at all, and the CS will instead execute until it sees the
> > > > > MI_BATCH_BUFFER_END.
> > > > 
> > > > Oh. My. Gosh. There's a BB_START?!!!
> > > 
> > > Looks like ;) At least my 830 seems perfectly happy with it. Well,
> > > as happy as it's ever been. Though I still couldn't get it complete
> > > a piglit run without hanging last I tried.
> > > 
> > > > 
> > > > > We still need the batch length since we do the CS TLB workaround
> > > > > and copy the batch into the permanently pinned scratch object
> > > > > and execute it from there. But for this we can simply use the
> > > > > batch object length when the user hasn't specified the actual
> > > > > batch length. So specifying the batch length becomes just a
> > > > > way to optimize the batch copy a little bit.
> > > > > 
> > > > > We lost batch_len from a bunch of igts (including the quiesce batch)
> > > > > so without this igt is utterly broken on 830/845. Also some igts such
> > > > > as gem_cpu_reloc never specified the batch_len and so didn't work.
> > > > > With MI_BATCH_BUFFER_START we don't have to fix up igt every time
> > > > > someone forgets that 830/845 exist.
> > > > > 
> > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Looks sane.
> > 
> > Checked against bspec, and it does indeed list BB_START for 830/845 and
> > we already comply with the errata.

Yeah, the coloring stuff should be enough.

> 
> The other question, does this obsolete our work around? Can I be that
> optimisitic?

The CS TLB one? I think I tried it at some point, and things till
failed. But stuff fails even with the w/a (like I said piglit will
hang the GPU eventually), so I can't be sure that I actually tested
the CS TLB fail. I think I need to retest at some point.

As far as the docs go, I only remember it mentioning some TLB fail
affecting the blitter. I guess the CS TLB fail isn't actually
documented anywhere?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 10/10] drm/i915: Use MI_BATCH_BUFFER_START on 830/845

[Chris Wilson]

On Tue, Dec 15, 2015 at 01:05:56PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 15, 2015 at 10:24:13AM +0000, Chris Wilson wrote:
> > The other question, does this obsolete our work around? Can I be that
> > optimisitic?
> 
> The CS TLB one? I think I tried it at some point, and things till
> failed. But stuff fails even with the w/a (like I said piglit will
> hang the GPU eventually), so I can't be sure that I actually tested
> the CS TLB fail. I think I need to retest at some point.
> 
> As far as the docs go, I only remember it mentioning some TLB fail
> affecting the blitter. I guess the CS TLB fail isn't actually
> documented anywhere?

It's hard to be sure since the issue is only mentioned obliquely in
bspec. I strongly suspect there is only one set of TLB on the device, so
I think it is the same. But I never did figure out what flush they
meant, as all the chipset or MI level flushes never seemed to do anything
to improve the situation.
-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: [PATCH 10/10] drm/i915: Use MI_BATCH_BUFFER_START on 830/845

[Ville Syrjälä]

On Tue, Dec 15, 2015 at 11:22:29AM +0000, Chris Wilson wrote:
> On Tue, Dec 15, 2015 at 01:05:56PM +0200, Ville Syrjälä wrote:
> > On Tue, Dec 15, 2015 at 10:24:13AM +0000, Chris Wilson wrote:
> > > The other question, does this obsolete our work around? Can I be that
> > > optimisitic?
> > 
> > The CS TLB one? I think I tried it at some point, and things till
> > failed. But stuff fails even with the w/a (like I said piglit will
> > hang the GPU eventually), so I can't be sure that I actually tested
> > the CS TLB fail. I think I need to retest at some point.
> > 
> > As far as the docs go, I only remember it mentioning some TLB fail
> > affecting the blitter. I guess the CS TLB fail isn't actually
> > documented anywhere?
> 
> It's hard to be sure since the issue is only mentioned obliquely in
> bspec. I strongly suspect there is only one set of TLB on the device, so
> I think it is the same. But I never did figure out what flush they
> meant, as all the chipset or MI level flushes never seemed to do anything
> to improve the situation.

Programming Environment 1.4.9.4 claims that there are several TLBs. But
not sure if that really holds for all devices.

It also has the following table:

GTT TLBs
TLB            | Normal Invalidation Mechanism  | Invalidated by Page Table Enable bit
               |                                | of PGTBL_CTL register?
...
Command Stream | Through a Page Table PTE write | Yes

Which might hint that PGTBL_CTL might be the way to force invalidate
them. But IIRC you may have once said that you already tried it. In any
case, even if it would work we'd need to make sure no GTT access is
happening when toggling the bit (assuming we'd have to toggle it).

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ failure: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on a90796840c30dac6d9907439bf98d1d08046c49d drm-intel-nightly: 2016y-01m-11d-17h-22m-54s UTC integration manifest

Test gem_storedw_loop:
        Subgroup basic-render:
                pass       -> DMESG-WARN (skl-i5k-2) UNSTABLE
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (ivb-t430s)
                pass       -> FAIL       (bdw-ultra)
                pass       -> FAIL       (byt-nuc)
                pass       -> FAIL       (snb-x220t)
                pass       -> FAIL       (snb-dellxps)
                pass       -> FAIL       (hsw-brixbox)
                pass       -> FAIL       (ilk-hp8440p)
        Subgroup hang-read-crc-pipe-b:
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (ivb-t430s)
                pass       -> FAIL       (byt-nuc)
                pass       -> FAIL       (snb-x220t)
                pass       -> FAIL       (snb-dellxps)
                pass       -> FAIL       (ilk-hp8440p)
        Subgroup hang-read-crc-pipe-c:
                pass       -> DMESG-WARN (bsw-nuc-2)
                pass       -> FAIL       (skl-i5k-2)
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (skl-i7k-2)
                pass       -> FAIL       (ivb-t430s)
                pass       -> FAIL       (hsw-brixbox)
                pass       -> FAIL       (bdw-nuci7)
        Subgroup nonblocking-crc-pipe-a:
                pass       -> FAIL       (skl-i5k-2)
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (bdw-ultra)
                pass       -> FAIL       (skl-i7k-2)
                pass       -> FAIL       (byt-nuc)
                pass       -> FAIL       (snb-x220t)
                pass       -> FAIL       (snb-dellxps)
                pass       -> FAIL       (hsw-brixbox)
                pass       -> FAIL       (bdw-nuci7)
                pass       -> FAIL       (ilk-hp8440p)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (bdw-ultra)
                pass       -> FAIL       (byt-nuc)
                pass       -> FAIL       (snb-x220t)
                pass       -> FAIL       (snb-dellxps)
                pass       -> FAIL       (hsw-brixbox)
                pass       -> FAIL       (bdw-nuci7)
                pass       -> FAIL       (ilk-hp8440p)
        Subgroup nonblocking-crc-pipe-b:
                dmesg-warn -> DMESG-FAIL (snb-x220t)
                dmesg-warn -> DMESG-FAIL (snb-dellxps)
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (ilk-hp8440p)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> FAIL       (snb-x220t)
                pass       -> FAIL       (snb-dellxps)
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (ilk-hp8440p)
        Subgroup nonblocking-crc-pipe-c:
                pass       -> FAIL       (hsw-gt2)
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                pass       -> FAIL       (bsw-nuc-2)
                pass       -> FAIL       (skl-i5k-2)
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (skl-i7k-2)
                pass       -> FAIL       (hsw-brixbox)
                pass       -> FAIL       (bdw-nuci7)
        Subgroup read-crc-pipe-a:
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (ivb-t430s)
                pass       -> FAIL       (bdw-ultra)
                pass       -> FAIL       (skl-i7k-2)
                pass       -> FAIL       (byt-nuc)
                pass       -> FAIL       (snb-x220t)
                pass       -> FAIL       (snb-dellxps)
                pass       -> FAIL       (hsw-brixbox)
                pass       -> FAIL       (bdw-nuci7)
                pass       -> FAIL       (ilk-hp8440p)
        Subgroup read-crc-pipe-a-frame-sequence:
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (ivb-t430s)
                pass       -> FAIL       (bdw-ultra)
                pass       -> FAIL       (byt-nuc)
                pass       -> FAIL       (snb-x220t)
                pass       -> FAIL       (snb-dellxps)
                pass       -> FAIL       (hsw-brixbox)
                pass       -> FAIL       (bdw-nuci7)
                pass       -> FAIL       (ilk-hp8440p)
        Subgroup read-crc-pipe-b:
                dmesg-warn -> DMESG-FAIL (snb-x220t)
                dmesg-warn -> DMESG-FAIL (snb-dellxps)
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (ilk-hp8440p)
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> FAIL       (snb-x220t)
                pass       -> FAIL       (snb-dellxps)
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (ilk-hp8440p)
                pass       -> FAIL       (ivb-t430s)
        Subgroup read-crc-pipe-c:
                pass       -> FAIL       (hsw-gt2)
        Subgroup read-crc-pipe-c-frame-sequence:
                dmesg-warn -> FAIL       (bsw-nuc-2)
                pass       -> FAIL       (skl-i5k-2)
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (skl-i7k-2)
                pass       -> FAIL       (ivb-t430s)
                pass       -> FAIL       (hsw-brixbox)
                pass       -> FAIL       (bdw-nuci7)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> FAIL       (skl-i5k-2)
                pass       -> FAIL       (hsw-gt2)
                pass       -> FAIL       (snb-x220t)
                pass       -> FAIL       (snb-dellxps)
                pass       -> FAIL       (hsw-brixbox)
                pass       -> FAIL       (ilk-hp8440p)
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> DMESG-FAIL (snb-x220t)
                dmesg-warn -> DMESG-FAIL (snb-dellxps)
                dmesg-warn -> DMESG-FAIL (ilk-hp8440p)

bdw-nuci7        total:138  pass:122  dwarn:0   dfail:0   fail:7   skip:9  
bdw-ultra        total:138  pass:126  dwarn:0   dfail:0   fail:6   skip:6  
bsw-nuc-2        total:141  pass:113  dwarn:2   dfail:0   fail:2   skip:24 
byt-nuc          total:141  pass:117  dwarn:3   dfail:0   fail:6   skip:15 
hsw-brixbox      total:141  pass:125  dwarn:0   dfail:0   fail:9   skip:7  
hsw-gt2          total:141  pass:121  dwarn:0   dfail:0   fail:16  skip:4  
ilk-hp8440p      total:141  pass:90   dwarn:2   dfail:1   fail:11  skip:37 
ivb-t430s        total:135  pass:115  dwarn:3   dfail:4   fail:7   skip:6  
skl-i5k-2        total:141  pass:126  dwarn:2   dfail:0   fail:5   skip:8  
skl-i7k-2        total:141  pass:127  dwarn:1   dfail:0   fail:5   skip:8  
snb-dellxps      total:141  pass:113  dwarn:2   dfail:3   fail:9   skip:14 
snb-x220t        total:141  pass:113  dwarn:2   dfail:3   fail:10  skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1137/

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/10] drm/i915: Fixes from my attempt at running igt on gen2

[Ville Syrjälä]

On Mon, Dec 14, 2015 at 06:23:39PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> It's been a while since I last ran igt on gen2, so I figured I'd
> give it a shot. 855 had some failures, 830 no longer worked at
> all. So I went ahead and fixed them, and here's the result.
> 
> The first three patches are not even gen2 specific bugs I caught
> with this effort. The rest is for gen2.
> 
> I have some fixes for igt as well, which I'll post separately.
> 
> The good news is that with these patches (and the igt fixes) my
> 855 completes a full kms_flip run without failures, and the BAT
> set has only one failure (gem_render_tiled_blits). 830 is fairly
> good too, but it does have a lot of underruns and pipe_assert()
> dmesg warnings. Lot of those are due to the pipe enable quirks
> since we implement those quite haphazardly.
> 
> The series is available here:
> git://github.com/vsyrjala/linux.git gen2_igt_fixes
> 
> Ville Syrjälä (10):
>   drm/i915: Cleanup phys status page too
>   drm/i915: Wait for pipe to start before sampling vblank timestamps on
>     gen2
>   drm/i915: Allow 27 bytes child_dev for VBT <109
>   drm/i915: Expect child dev size of 22 bytes for VBT < 106
>   drm/i915: Use MI_BATCH_BUFFER_START on 830/845

Merged these. Thanks for the reviews.

>   drm/i915: Release mmaps on partial ggtt vma unbind
>   drm/i915: Write out crc frame counts in hex
>   drm/i915: Use drm_vblank_count() on gen2 for crc frame count
>   drm/i915: Enable vblank_disable_immediate on gen2
>   drm/i915: Reject < 8 byte batches on 830/845

And at some point I'll need to figure out what to do with the
rest. Some I'll just drop for sure.

> 
>  drivers/gpu/drm/i915/i915_debugfs.c        | 13 ++++++++++++-
>  drivers/gpu/drm/i915/i915_gem.c            |  3 +++
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  3 +++
>  drivers/gpu/drm/i915/i915_irq.c            | 14 +++++---------
>  drivers/gpu/drm/i915/intel_bios.c          | 21 ++++++++++++--------
>  drivers/gpu/drm/i915/intel_display.c       | 11 +++++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.c    | 31 +++++++++++++++++++++++-------
>  7 files changed, 71 insertions(+), 25 deletions(-)
> 
> -- 
> 2.4.10

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/10] drm/i915: Fixes from my attempt at running igt on gen2

[Daniel Vetter]

On Tue, Jan 12, 2016 at 04:54:27PM +0200, Ville Syrjälä wrote:
> On Mon, Dec 14, 2015 at 06:23:39PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > It's been a while since I last ran igt on gen2, so I figured I'd
> > give it a shot. 855 had some failures, 830 no longer worked at
> > all. So I went ahead and fixed them, and here's the result.
> > 
> > The first three patches are not even gen2 specific bugs I caught
> > with this effort. The rest is for gen2.
> > 
> > I have some fixes for igt as well, which I'll post separately.
> > 
> > The good news is that with these patches (and the igt fixes) my
> > 855 completes a full kms_flip run without failures, and the BAT
> > set has only one failure (gem_render_tiled_blits). 830 is fairly
> > good too, but it does have a lot of underruns and pipe_assert()
> > dmesg warnings. Lot of those are due to the pipe enable quirks
> > since we implement those quite haphazardly.
> > 
> > The series is available here:
> > git://github.com/vsyrjala/linux.git gen2_igt_fixes
> > 
> > Ville Syrjälä (10):
> >   drm/i915: Cleanup phys status page too
> >   drm/i915: Wait for pipe to start before sampling vblank timestamps on
> >     gen2
> >   drm/i915: Allow 27 bytes child_dev for VBT <109
> >   drm/i915: Expect child dev size of 22 bytes for VBT < 106
> >   drm/i915: Use MI_BATCH_BUFFER_START on 830/845
> 
> Merged these. Thanks for the reviews.

CI seems extremely unhappy about your series. Haven't checked whether it's
something else or your stuff, but since we've decided last week that BAT
CI approval is required such a patch series shouldn't be merged any more.

Ok meanwhile I think since we're still just ramping up, but still.
-Daniel

> 
> >   drm/i915: Release mmaps on partial ggtt vma unbind
> >   drm/i915: Write out crc frame counts in hex
> >   drm/i915: Use drm_vblank_count() on gen2 for crc frame count
> >   drm/i915: Enable vblank_disable_immediate on gen2
> >   drm/i915: Reject < 8 byte batches on 830/845
> 
> And at some point I'll need to figure out what to do with the
> rest. Some I'll just drop for sure.
> 
> > 
> >  drivers/gpu/drm/i915/i915_debugfs.c        | 13 ++++++++++++-
> >  drivers/gpu/drm/i915/i915_gem.c            |  3 +++
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  3 +++
> >  drivers/gpu/drm/i915/i915_irq.c            | 14 +++++---------
> >  drivers/gpu/drm/i915/intel_bios.c          | 21 ++++++++++++--------
> >  drivers/gpu/drm/i915/intel_display.c       | 11 +++++++++++
> >  drivers/gpu/drm/i915/intel_ringbuffer.c    | 31 +++++++++++++++++++++++-------
> >  7 files changed, 71 insertions(+), 25 deletions(-)
> > 
> > -- 
> > 2.4.10
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx