<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> Tested-By: Mikita Lipski <mikita.lipski@amd.com> Thanks, Nick </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Grodzovsky, Andrey Sent: Monday, January 22, 2018 1:36:01 PM To: Wentland, Harry; Alex Deucher; amd-gfx@lists.freedesktop.org; Lipski, Mikita Cc: Deucher, Alexander Subject: Re: [PATCH] drm/amdgpu: drop the drm irq pre/post/un install callbacks <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">+ Mikita who verified the patch. Andrey On 01/22/2018 01:34 PM, Harry Wentland wrote: > On 2018-01-22 12:22 PM, Alex Deucher wrote: >> The preinstall callback didn't do anything because not all >> of the IPs were initialized when it was called. >> >> Move the postinstall setup into sequence in the driver. >> >> The uninstall callback disabled all interrupt source, but >> it got called too late in the driver sequence and caused problems >> with IPs who already freed the relevant data structures. Move >> the call into the right place in the driver sequence. >> >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > Acked-by: Harry Wentland <harry.wentland@amd.com> > > Harry > >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++ >> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 -- >> drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 52 ++---------------------------- >> drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h | 4 +-- >> 4 files changed, 6 insertions(+), 56 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> index d09c4ee9f7e1..02a9157e72ac 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> @@ -1483,6 +1483,9 @@ static int amdgpu_device_ip_fini(struct amdgpu_device *adev) >> adev->ip_blocks[i].status.hw = false; >> } >> >> + /* disable all interrupts */ >> + amdgpu_irq_disable_all(adev); >> + >> for (i = adev->num_ip_blocks - 1; i >= 0; i--) { >> if (!adev->ip_blocks[i].status.sw) >> continue; >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> index 24bc3e9203e2..ab4cb4917565 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >> @@ -856,9 +856,6 @@ static struct drm_driver kms_driver = { >> .disable_vblank = amdgpu_disable_vblank_kms, >> .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, >> .get_scanout_position = amdgpu_get_crtc_scanout_position, >> - .irq_preinstall = amdgpu_irq_preinstall, >> - .irq_postinstall = amdgpu_irq_postinstall, >> - .irq_uninstall = amdgpu_irq_uninstall, >> .irq_handler = amdgpu_irq_handler, >> .ioctls = amdgpu_ioctls_kms, >> .gem_free_object_unlocked = amdgpu_gem_object_free, >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c >> index 56bcd59c3399..f6f2a662bb8f 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c >> @@ -92,7 +92,7 @@ static void amdgpu_irq_reset_work_func(struct work_struct *work) >> } >> >> /* Disable *all* interrupts */ >> -static void amdgpu_irq_disable_all(struct amdgpu_device *adev) >> +void amdgpu_irq_disable_all(struct amdgpu_device *adev) >> { >> unsigned long irqflags; >> unsigned i, j, k; >> @@ -123,55 +123,6 @@ static void amdgpu_irq_disable_all(struct amdgpu_device *adev) >> } >> >> /** >> - * amdgpu_irq_preinstall - drm irq preinstall callback >> - * >> - * @dev: drm dev pointer >> - * >> - * Gets the hw ready to enable irqs (all asics). >> - * This function disables all interrupt sources on the GPU. >> - */ >> -void amdgpu_irq_preinstall(struct drm_device *dev) >> -{ >> - struct amdgpu_device *adev = dev->dev_private; >> - >> - /* Disable *all* interrupts */ >> - amdgpu_irq_disable_all(adev); >> - /* Clear bits */ >> - amdgpu_ih_process(adev); >> -} >> - >> -/** >> - * amdgpu_irq_postinstall - drm irq preinstall callback >> - * >> - * @dev: drm dev pointer >> - * >> - * Handles stuff to be done after enabling irqs (all asics). >> - * Returns 0 on success. >> - */ >> -int amdgpu_irq_postinstall(struct drm_device *dev) >> -{ >> - dev->max_vblank_count = 0x00ffffff; >> - return 0; >> -} >> - >> -/** >> - * amdgpu_irq_uninstall - drm irq uninstall callback >> - * >> - * @dev: drm dev pointer >> - * >> - * This function disables all interrupt sources on the GPU (all asics). >> - */ >> -void amdgpu_irq_uninstall(struct drm_device *dev) >> -{ >> - struct amdgpu_device *adev = dev->dev_private; >> - >> - if (adev == NULL) { >> - return; >> - } >> - amdgpu_irq_disable_all(adev); >> -} >> - >> -/** >> * amdgpu_irq_handler - irq handler >> * >> * @int irq, void *arg: args >> @@ -261,6 +212,7 @@ int amdgpu_irq_init(struct amdgpu_device *adev) >> cancel_work_sync(&adev->reset_work); >> return r; >> } >> + adev->ddev->max_vblank_count = 0x00ffffff; >> >> DRM_DEBUG("amdgpu: irq initialized.\n"); >> return 0; >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h >> index 0610cc4a9788..3375ad778edc 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h >> @@ -78,9 +78,7 @@ struct amdgpu_irq { >> uint32_t srbm_soft_reset; >> }; >> >> -void amdgpu_irq_preinstall(struct drm_device *dev); >> -int amdgpu_irq_postinstall(struct drm_device *dev); >> -void amdgpu_irq_uninstall(struct drm_device *dev); >> +void amdgpu_irq_disable_all(struct amdgpu_device *adev); >> irqreturn_t amdgpu_irq_handler(int irq, void *arg); >> >> int amdgpu_irq_init(struct amdgpu_device *adev); >> > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> </div> </div> </body> </html>