All of lore.kernel.org
 help / color / mirror / Atom feed
From: Inki Dae <inki.dae@samsung.com>
To: Andrzej Hajda <a.hajda@samsung.com>
Cc: "moderated list:ARM/S5P EXYNOS AR..."
	<linux-samsung-soc@vger.kernel.org>,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	open list <linux-kernel@vger.kernel.org>,
	dri-devel@lists.freedesktop.org,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence
Date: Fri, 12 Sep 2014 17:34:50 +0900	[thread overview]
Message-ID: <5412B02A.6040609@samsung.com> (raw)
In-Reply-To: <1410268573-2297-6-git-send-email-a.hajda@samsung.com>

Hi Andrzej,

On 2014년 09월 09일 22:16, Andrzej Hajda wrote:
> Adding reference to framebuffer should be accompanied with removing
> reference to old framebuffer assigned to the plane.
> This patch removes following warning:
> 
> [   95.038017] WARNING: CPU: 1 PID: 3067 at drivers/gpu/drm/drm_crtc.c:5115 drm_mode_config_cleanup+0x258/0x268()
> [   95.048086] Modules linked in:
> [   95.051430] CPU: 1 PID: 3067 Comm: bash Tainted: G        W      3.16.0-11355-g7a6eca5-dirty #3015
> [   95.060058] [<c0013e90>] (unwind_backtrace) from [<c0011128>] (show_stack+0x10/0x14)
> [   95.067766] [<c0011128>] (show_stack) from [<c04a5dc4>] (dump_stack+0x70/0xbc)
> [   95.074953] [<c04a5dc4>] (dump_stack) from [<c0021784>] (warn_slowpath_common+0x64/0x88)
> [   95.083005] [<c0021784>] (warn_slowpath_common) from [<c00217c4>] (warn_slowpath_null+0x1c/0x24)
> [   95.091780] [<c00217c4>] (warn_slowpath_null) from [<c0275fa0>] (drm_mode_config_cleanup+0x258/0x268)
> [   95.100983] [<c0275fa0>] (drm_mode_config_cleanup) from [<c0281724>] (exynos_drm_unload+0x38/0x50)
> [   95.109915] [<c0281724>] (exynos_drm_unload) from [<c026e248>] (drm_dev_unregister+0x24/0x98)
> [   95.118414] [<c026e248>] (drm_dev_unregister) from [<c026ecb0>] (drm_put_dev+0x28/0x64)
> [   95.126412] [<c026ecb0>] (drm_put_dev) from [<c02947c4>] (take_down_master+0x24/0x44)
> [   95.134218] [<c02947c4>] (take_down_master) from [<c0294870>] (component_del+0x8c/0xc8)
> [   95.142201] [<c0294870>] (component_del) from [<c0286c10>] (exynos_dsi_remove+0x18/0x2c)
> [   95.150294] [<c0286c10>] (exynos_dsi_remove) from [<c0299e04>] (platform_drv_remove+0x18/0x1c)
> [   95.158872] [<c0299e04>] (platform_drv_remove) from [<c02987a4>] (__device_release_driver+0x70/0xc4)
> [   95.167981] [<c02987a4>] (__device_release_driver) from [<c0298818>] (device_release_driver+0x20/0x2c)
> [   95.177268] [<c0298818>] (device_release_driver) from [<c02979d0>] (unbind_store+0x5c/0x94)
> [   95.185597] [<c02979d0>] (unbind_store) from [<c0297278>] (drv_attr_store+0x20/0x2c)
> [   95.193323] [<c0297278>] (drv_attr_store) from [<c012aa90>] (sysfs_kf_write+0x4c/0x50)
> [   95.201224] [<c012aa90>] (sysfs_kf_write) from [<c0129e94>] (kernfs_fop_write+0xc4/0x184)
> [   95.209393] [<c0129e94>] (kernfs_fop_write) from [<c00d03ec>] (vfs_write+0xa0/0x1a8)
> [   95.217111] [<c00d03ec>] (vfs_write) from [<c00d07f8>] (SyS_write+0x40/0x8c)
> [   95.224146] [<c00d07f8>] (SyS_write) from [<c000e660>] (ret_fast_syscall+0x0/0x48)
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index b68e58f..bde19f4 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -145,10 +145,16 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
>  	if (ret)
>  		return ret;
>  
> +	/* we need to unreference current fb after replacing it with new one */
> +	old_fb = plane->fb;
> +
>  	plane->crtc = crtc;
>  	plane->fb = crtc->primary->fb;
>  	drm_framebuffer_reference(plane->fb);
>  
> +	if (old_fb)
> +		drm_framebuffer_unreference(old_fb);

This time would be a good chance that we can consider drm flip queue to
make sure that whole memory region to old_fb is scanned out completely
before dropping a reference of old_fb. the reference of old_fb should be
dropped at irq handler of each crtc devices, fimd and mixer.

Thanks,
Inki Dae

> +
>  	return 0;
>  }
>  
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Inki Dae <inki.dae@samsung.com>
To: Andrzej Hajda <a.hajda@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	Joonyoung Shim <jy0922.shim@samsung.com>,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	dri-devel@lists.freedesktop.org,
	open list <linux-kernel@vger.kernel.org>,
	"moderated list:ARM/S5P EXYNOS AR..." 
	<linux-samsung-soc@vger.kernel.org>
Subject: Re: [PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence
Date: Fri, 12 Sep 2014 17:34:50 +0900	[thread overview]
Message-ID: <5412B02A.6040609@samsung.com> (raw)
In-Reply-To: <1410268573-2297-6-git-send-email-a.hajda@samsung.com>

Hi Andrzej,

On 2014년 09월 09일 22:16, Andrzej Hajda wrote:
> Adding reference to framebuffer should be accompanied with removing
> reference to old framebuffer assigned to the plane.
> This patch removes following warning:
> 
> [   95.038017] WARNING: CPU: 1 PID: 3067 at drivers/gpu/drm/drm_crtc.c:5115 drm_mode_config_cleanup+0x258/0x268()
> [   95.048086] Modules linked in:
> [   95.051430] CPU: 1 PID: 3067 Comm: bash Tainted: G        W      3.16.0-11355-g7a6eca5-dirty #3015
> [   95.060058] [<c0013e90>] (unwind_backtrace) from [<c0011128>] (show_stack+0x10/0x14)
> [   95.067766] [<c0011128>] (show_stack) from [<c04a5dc4>] (dump_stack+0x70/0xbc)
> [   95.074953] [<c04a5dc4>] (dump_stack) from [<c0021784>] (warn_slowpath_common+0x64/0x88)
> [   95.083005] [<c0021784>] (warn_slowpath_common) from [<c00217c4>] (warn_slowpath_null+0x1c/0x24)
> [   95.091780] [<c00217c4>] (warn_slowpath_null) from [<c0275fa0>] (drm_mode_config_cleanup+0x258/0x268)
> [   95.100983] [<c0275fa0>] (drm_mode_config_cleanup) from [<c0281724>] (exynos_drm_unload+0x38/0x50)
> [   95.109915] [<c0281724>] (exynos_drm_unload) from [<c026e248>] (drm_dev_unregister+0x24/0x98)
> [   95.118414] [<c026e248>] (drm_dev_unregister) from [<c026ecb0>] (drm_put_dev+0x28/0x64)
> [   95.126412] [<c026ecb0>] (drm_put_dev) from [<c02947c4>] (take_down_master+0x24/0x44)
> [   95.134218] [<c02947c4>] (take_down_master) from [<c0294870>] (component_del+0x8c/0xc8)
> [   95.142201] [<c0294870>] (component_del) from [<c0286c10>] (exynos_dsi_remove+0x18/0x2c)
> [   95.150294] [<c0286c10>] (exynos_dsi_remove) from [<c0299e04>] (platform_drv_remove+0x18/0x1c)
> [   95.158872] [<c0299e04>] (platform_drv_remove) from [<c02987a4>] (__device_release_driver+0x70/0xc4)
> [   95.167981] [<c02987a4>] (__device_release_driver) from [<c0298818>] (device_release_driver+0x20/0x2c)
> [   95.177268] [<c0298818>] (device_release_driver) from [<c02979d0>] (unbind_store+0x5c/0x94)
> [   95.185597] [<c02979d0>] (unbind_store) from [<c0297278>] (drv_attr_store+0x20/0x2c)
> [   95.193323] [<c0297278>] (drv_attr_store) from [<c012aa90>] (sysfs_kf_write+0x4c/0x50)
> [   95.201224] [<c012aa90>] (sysfs_kf_write) from [<c0129e94>] (kernfs_fop_write+0xc4/0x184)
> [   95.209393] [<c0129e94>] (kernfs_fop_write) from [<c00d03ec>] (vfs_write+0xa0/0x1a8)
> [   95.217111] [<c00d03ec>] (vfs_write) from [<c00d07f8>] (SyS_write+0x40/0x8c)
> [   95.224146] [<c00d07f8>] (SyS_write) from [<c000e660>] (ret_fast_syscall+0x0/0x48)
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index b68e58f..bde19f4 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -145,10 +145,16 @@ exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
>  	if (ret)
>  		return ret;
>  
> +	/* we need to unreference current fb after replacing it with new one */
> +	old_fb = plane->fb;
> +
>  	plane->crtc = crtc;
>  	plane->fb = crtc->primary->fb;
>  	drm_framebuffer_reference(plane->fb);
>  
> +	if (old_fb)
> +		drm_framebuffer_unreference(old_fb);

This time would be a good chance that we can consider drm flip queue to
make sure that whole memory region to old_fb is scanned out completely
before dropping a reference of old_fb. the reference of old_fb should be
dropped at irq handler of each crtc devices, fimd and mixer.

Thanks,
Inki Dae

> +
>  	return 0;
>  }
>  
> 


  reply	other threads:[~2014-09-12  8:34 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-09 13:16 [PATCH 0/9] drm/exynos: initialization/deinitialization fixes Andrzej Hajda
2014-09-09 13:16 ` Andrzej Hajda
2014-09-09 13:16 ` [PATCH 1/9] drm/exynos/ipp: traverse ipp drivers list safely Andrzej Hajda
2014-09-09 13:16   ` Andrzej Hajda
2014-09-09 13:16 ` [PATCH 2/9] drm/exynos: fix drm driver de-initialization order Andrzej Hajda
2014-09-09 13:16   ` Andrzej Hajda
2014-09-09 13:16 ` [PATCH 3/9] drm/exynos/fbdev: fix fbdev gem object cleanup Andrzej Hajda
2014-09-09 13:16   ` Andrzej Hajda
2014-09-09 13:16 ` [PATCH 4/9] drm/exynos/fb: free exynos framebuffer on error Andrzej Hajda
2014-09-09 13:16   ` Andrzej Hajda
2014-09-09 13:16 ` [PATCH 5/9] drm/exynos/crtc: fix framebuffer reference sequence Andrzej Hajda
2014-09-09 13:16   ` Andrzej Hajda
2014-09-12  8:34   ` Inki Dae [this message]
2014-09-12  8:34     ` Inki Dae
2014-09-12  8:57     ` Daniel Vetter
2014-09-12  8:57       ` Daniel Vetter
2014-09-12  9:21       ` Inki Dae
2014-09-12  9:21         ` Inki Dae
2014-09-12  9:27       ` Andrzej Hajda
2014-09-12  9:27         ` Andrzej Hajda
2014-09-12 10:45         ` Inki Dae
2014-09-12 11:04           ` Andrzej Hajda
2014-09-12 11:24             ` Inki Dae
2014-09-12 14:03         ` Daniel Vetter
2014-09-12 14:03           ` Daniel Vetter
2014-09-09 13:16 ` [PATCH 6/9] drm/exynos/dsi: unregister connector on removal Andrzej Hajda
2014-09-09 13:16   ` Andrzej Hajda
2014-09-09 13:16 ` [PATCH 7/9] drm/exynos/dpi: unregister connector and panel " Andrzej Hajda
2014-09-09 13:16   ` Andrzej Hajda
2014-09-09 13:16 ` [PATCH 8/9] drm/exynos/dp: unregister connector " Andrzej Hajda
2014-09-09 13:16   ` Andrzej Hajda
2014-09-09 13:16 ` [PATCH 9/9] drm/exynos/hdmi: " Andrzej Hajda
2014-09-09 13:16   ` Andrzej Hajda
2014-09-17 11:44 ` [PATCH 0/9] drm/exynos: initialization/deinitialization fixes Inki Dae

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5412B02A.6040609@samsung.com \
    --to=inki.dae@samsung.com \
    --cc=a.hajda@samsung.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=sw0312.kim@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.