* [PATCH 1/2] drm/amdgpu:fix virtual dce bug
@ 2017-11-16 3:14 Monk Liu
[not found] ` <1510802063-20869-1-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Monk Liu @ 2017-11-16 3:14 UTC (permalink / raw
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Monk Liu
this fix the issue that access memory after freed
after driver unloaded.
Change-Id: I64e2488c18f5dc044b57c74567785da21fc028da
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index a8829af..39460eb 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -437,6 +437,8 @@ static int dce_virtual_sw_fini(void *handle)
drm_kms_helper_poll_fini(adev->ddev);
drm_mode_config_cleanup(adev->ddev);
+ /* clear crtcs pointer to avoid dce irq finish routine access freed data */
+ memset(adev->mode_info.crtcs, 0, sizeof(adev->mode_info.crtcs[0]) * AMDGPU_MAX_CRTCS);
adev->mode_info.mode_config_initialized = false;
return 0;
}
@@ -723,7 +725,7 @@ static void dce_virtual_set_crtc_vblank_interrupt_state(struct amdgpu_device *ad
int crtc,
enum amdgpu_interrupt_state state)
{
- if (crtc >= adev->mode_info.num_crtc) {
+ if (crtc >= adev->mode_info.num_crtc || !adev->mode_info.crtcs[crtc]) {
DRM_DEBUG("invalid crtc %d\n", crtc);
return;
}
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] drm/amdgpu:cancel timer of virtual DCE
[not found] ` <1510802063-20869-1-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org>
@ 2017-11-16 3:14 ` Monk Liu
[not found] ` <1510802063-20869-2-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org>
2017-11-16 16:40 ` [PATCH 1/2] drm/amdgpu:fix virtual dce bug Jan Vesely
1 sibling, 1 reply; 7+ messages in thread
From: Monk Liu @ 2017-11-16 3:14 UTC (permalink / raw
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Monk Liu
virtual DCE Timer structure is already released
after its sw_fini(), so we need to cancel the
its Timer in hw_fini() otherwise the Timer canceling
is missed.
Change-Id: I03d6ca7aa07591d287da379ef4fe008f06edaff6
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index 39460eb..7438491 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -489,8 +489,21 @@ static int dce_virtual_hw_init(void *handle)
return 0;
}
+static void dce_virtual_set_crtc_vblank_interrupt_state(struct amdgpu_device *adev,
+ int crtc,
+ enum amdgpu_interrupt_state state);
+
static int dce_virtual_hw_fini(void *handle)
{
+ struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+ int i = 0;
+
+ while (i < AMDGPU_MAX_CRTCS) {
+ if (adev->mode_info.crtcs[i])
+ dce_virtual_set_crtc_vblank_interrupt_state(adev, i, AMDGPU_IRQ_STATE_DISABLE);
+ i++;
+ }
+
return 0;
}
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu:cancel timer of virtual DCE
[not found] ` <1510802063-20869-2-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org>
@ 2017-11-16 4:13 ` Deucher, Alexander
[not found] ` <BN6PR12MB165204AC0F92A06F56BDC6ACF72E0-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Deucher, Alexander @ 2017-11-16 4:13 UTC (permalink / raw
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org; +Cc: Liu, Monk
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Monk Liu
> Sent: Wednesday, November 15, 2017 10:14 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Liu, Monk
> Subject: [PATCH 2/2] drm/amdgpu:cancel timer of virtual DCE
>
> virtual DCE Timer structure is already released
> after its sw_fini(), so we need to cancel the
> its Timer in hw_fini() otherwise the Timer canceling
> is missed.
>
> Change-Id: I03d6ca7aa07591d287da379ef4fe008f06edaff6
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> index 39460eb..7438491 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> @@ -489,8 +489,21 @@ static int dce_virtual_hw_init(void *handle)
> return 0;
> }
>
> +static void dce_virtual_set_crtc_vblank_interrupt_state(struct
> amdgpu_device *adev,
> + int crtc,
> + enum
> amdgpu_interrupt_state state);
> +
Please put the forward declaration at the top of the file.
> static int dce_virtual_hw_fini(void *handle)
> {
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> + int i = 0;
> +
> + while (i < AMDGPU_MAX_CRTCS) {
> + if (adev->mode_info.crtcs[i])
> + dce_virtual_set_crtc_vblank_interrupt_state(adev, i,
> AMDGPU_IRQ_STATE_DISABLE);
> + i++;
> + }
I think a for loop is clearer here. Also, why not use adev->mode_info.num_crtc so we don’t loop longer than we have to?
Alex
> +
> return 0;
> }
>
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu:cancel timer of virtual DCE
[not found] ` <BN6PR12MB165204AC0F92A06F56BDC6ACF72E0-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2017-11-16 6:09 ` Liu, Monk
0 siblings, 0 replies; 7+ messages in thread
From: Liu, Monk @ 2017-11-16 6:09 UTC (permalink / raw
To: Deucher, Alexander,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
Good idea
-----Original Message-----
From: Deucher, Alexander
Sent: 2017年11月16日 12:14
To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Liu, Monk <Monk.Liu@amd.com>
Subject: RE: [PATCH 2/2] drm/amdgpu:cancel timer of virtual DCE
> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Monk Liu
> Sent: Wednesday, November 15, 2017 10:14 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Liu, Monk
> Subject: [PATCH 2/2] drm/amdgpu:cancel timer of virtual DCE
>
> virtual DCE Timer structure is already released after its sw_fini(),
> so we need to cancel the its Timer in hw_fini() otherwise the Timer
> canceling is missed.
>
> Change-Id: I03d6ca7aa07591d287da379ef4fe008f06edaff6
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> index 39460eb..7438491 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> @@ -489,8 +489,21 @@ static int dce_virtual_hw_init(void *handle)
> return 0;
> }
>
> +static void dce_virtual_set_crtc_vblank_interrupt_state(struct
> amdgpu_device *adev,
> + int crtc,
> + enum
> amdgpu_interrupt_state state);
> +
Please put the forward declaration at the top of the file.
> static int dce_virtual_hw_fini(void *handle) {
> + struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> + int i = 0;
> +
> + while (i < AMDGPU_MAX_CRTCS) {
> + if (adev->mode_info.crtcs[i])
> + dce_virtual_set_crtc_vblank_interrupt_state(adev, i,
> AMDGPU_IRQ_STATE_DISABLE);
> + i++;
> + }
I think a for loop is clearer here. Also, why not use adev->mode_info.num_crtc so we don’t loop longer than we have to?
Alex
> +
> return 0;
> }
>
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu:fix virtual dce bug
[not found] ` <1510802063-20869-1-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org>
2017-11-16 3:14 ` [PATCH 2/2] drm/amdgpu:cancel timer of virtual DCE Monk Liu
@ 2017-11-16 16:40 ` Jan Vesely
[not found] ` <1510850414.3218.161.camel-kgbqMDwikbSVc3sceRu5cw@public.gmane.org>
1 sibling, 1 reply; 7+ messages in thread
From: Jan Vesely @ 2017-11-16 16:40 UTC (permalink / raw
To: Monk Liu, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
[-- Attachment #1.1: Type: text/plain, Size: 1480 bytes --]
On Thu, 2017-11-16 at 11:14 +0800, Monk Liu wrote:
> this fix the issue that access memory after freed
> after driver unloaded.
can you please change the patch subject and commit message to something
more descriptive?
Jan
>
> Change-Id: I64e2488c18f5dc044b57c74567785da21fc028da
> Signed-off-by: Monk Liu <Monk.Liu-5C7GfCeVMHo@public.gmane.org>
> ---
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> index a8829af..39460eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> @@ -437,6 +437,8 @@ static int dce_virtual_sw_fini(void *handle)
> drm_kms_helper_poll_fini(adev->ddev);
>
> drm_mode_config_cleanup(adev->ddev);
> + /* clear crtcs pointer to avoid dce irq finish routine access freed data */
> + memset(adev->mode_info.crtcs, 0, sizeof(adev->mode_info.crtcs[0]) * AMDGPU_MAX_CRTCS);
> adev->mode_info.mode_config_initialized = false;
> return 0;
> }
> @@ -723,7 +725,7 @@ static void dce_virtual_set_crtc_vblank_interrupt_state(struct amdgpu_device *ad
> int crtc,
> enum amdgpu_interrupt_state state)
> {
> - if (crtc >= adev->mode_info.num_crtc) {
> + if (crtc >= adev->mode_info.num_crtc || !adev->mode_info.crtcs[crtc]) {
> DRM_DEBUG("invalid crtc %d\n", crtc);
> return;
> }
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 1/2] drm/amdgpu:fix virtual dce bug
[not found] ` <1510850414.3218.161.camel-kgbqMDwikbSVc3sceRu5cw@public.gmane.org>
@ 2017-11-17 4:26 ` Liu, Monk
[not found] ` <BLUPR12MB04498F5AE2FF155F75D61BF3842F0-7LeqcoF/hwpTIQvHjXdJlwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Liu, Monk @ 2017-11-17 4:26 UTC (permalink / raw
To: Jan Vesely,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
I think it's already clear enough
-----Original Message-----
From: Jan Vesely [mailto:jv356@scarletmail.rutgers.edu] On Behalf Of Jan Vesely
Sent: 2017年11月17日 0:40
To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/amdgpu:fix virtual dce bug
On Thu, 2017-11-16 at 11:14 +0800, Monk Liu wrote:
> this fix the issue that access memory after freed after driver
> unloaded.
can you please change the patch subject and commit message to something more descriptive?
Jan
>
> Change-Id: I64e2488c18f5dc044b57c74567785da21fc028da
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> index a8829af..39460eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> @@ -437,6 +437,8 @@ static int dce_virtual_sw_fini(void *handle)
> drm_kms_helper_poll_fini(adev->ddev);
>
> drm_mode_config_cleanup(adev->ddev);
> + /* clear crtcs pointer to avoid dce irq finish routine access freed data */
> + memset(adev->mode_info.crtcs, 0, sizeof(adev->mode_info.crtcs[0]) *
> +AMDGPU_MAX_CRTCS);
> adev->mode_info.mode_config_initialized = false;
> return 0;
> }
> @@ -723,7 +725,7 @@ static void dce_virtual_set_crtc_vblank_interrupt_state(struct amdgpu_device *ad
> int crtc,
> enum amdgpu_interrupt_state state) {
> - if (crtc >= adev->mode_info.num_crtc) {
> + if (crtc >= adev->mode_info.num_crtc ||
> +!adev->mode_info.crtcs[crtc]) {
> DRM_DEBUG("invalid crtc %d\n", crtc);
> return;
> }
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] drm/amdgpu:fix virtual dce bug
[not found] ` <BLUPR12MB04498F5AE2FF155F75D61BF3842F0-7LeqcoF/hwpTIQvHjXdJlwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2017-11-17 6:00 ` Jan Vesely
0 siblings, 0 replies; 7+ messages in thread
From: Jan Vesely @ 2017-11-17 6:00 UTC (permalink / raw
To: Liu, Monk,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
[-- Attachment #1.1: Type: text/plain, Size: 2259 bytes --]
On Fri, 2017-11-17 at 04:26 +0000, Liu, Monk wrote:
> I think it's already clear enough
nice. what a friendly response. good job!
"fix a bug" is definitely not descriptive of the change, and the commit
message does not even parse as a sentence.
Jan
>
> -----Original Message-----
> From: Jan Vesely [mailto:jv356-nChP9VK/LToysehGhLVACCmkPJUwDNjP@public.gmane.org] On Behalf Of Jan Vesely
> Sent: 2017年11月17日 0:40
> To: Liu, Monk <Monk.Liu-5C7GfCeVMHo@public.gmane.org>; amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> Subject: Re: [PATCH 1/2] drm/amdgpu:fix virtual dce bug
>
> On Thu, 2017-11-16 at 11:14 +0800, Monk Liu wrote:
> > this fix the issue that access memory after freed after driver
> > unloaded.
>
> can you please change the patch subject and commit message to something more descriptive?
>
> Jan
>
> >
> > Change-Id: I64e2488c18f5dc044b57c74567785da21fc028da
> > Signed-off-by: Monk Liu <Monk.Liu-5C7GfCeVMHo@public.gmane.org>
> > ---
> > drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> > b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> > index a8829af..39460eb 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> > @@ -437,6 +437,8 @@ static int dce_virtual_sw_fini(void *handle)
> > drm_kms_helper_poll_fini(adev->ddev);
> >
> > drm_mode_config_cleanup(adev->ddev);
> > + /* clear crtcs pointer to avoid dce irq finish routine access freed data */
> > + memset(adev->mode_info.crtcs, 0, sizeof(adev->mode_info.crtcs[0]) *
> > +AMDGPU_MAX_CRTCS);
> > adev->mode_info.mode_config_initialized = false;
> > return 0;
> > }
> > @@ -723,7 +725,7 @@ static void dce_virtual_set_crtc_vblank_interrupt_state(struct amdgpu_device *ad
> > int crtc,
> > enum amdgpu_interrupt_state state) {
> > - if (crtc >= adev->mode_info.num_crtc) {
> > + if (crtc >= adev->mode_info.num_crtc ||
> > +!adev->mode_info.crtcs[crtc]) {
> > DRM_DEBUG("invalid crtc %d\n", crtc);
> > return;
> > }
--
Jan Vesely <jan.vesely-kgbqMDwikbSVc3sceRu5cw@public.gmane.org>
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-11-17 6:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-16 3:14 [PATCH 1/2] drm/amdgpu:fix virtual dce bug Monk Liu
[not found] ` <1510802063-20869-1-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org>
2017-11-16 3:14 ` [PATCH 2/2] drm/amdgpu:cancel timer of virtual DCE Monk Liu
[not found] ` <1510802063-20869-2-git-send-email-Monk.Liu-5C7GfCeVMHo@public.gmane.org>
2017-11-16 4:13 ` Deucher, Alexander
[not found] ` <BN6PR12MB165204AC0F92A06F56BDC6ACF72E0-/b2+HYfkarQqUD6E6FAiowdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-11-16 6:09 ` Liu, Monk
2017-11-16 16:40 ` [PATCH 1/2] drm/amdgpu:fix virtual dce bug Jan Vesely
[not found] ` <1510850414.3218.161.camel-kgbqMDwikbSVc3sceRu5cw@public.gmane.org>
2017-11-17 4:26 ` Liu, Monk
[not found] ` <BLUPR12MB04498F5AE2FF155F75D61BF3842F0-7LeqcoF/hwpTIQvHjXdJlwdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2017-11-17 6:00 ` Jan Vesely
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.