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

[Daniel Vetter <daniel@ffwll.ch>]

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