Re: [PATCH 07/11] drm/amdgpu/gfx6: Properly enable/disable priv_req and priv_inst interrupts

["Timur Kristóf" <timur.kristof@gmail.com>]

On 2026. július 15., szerda 13:22:21 közép-európai nyári idő Tvrtko Ursulin 
wrote:
> On 15/07/2026 11:53, Timur Kristóf wrote:
> > On 2026. július 15., szerda 12:19:40 közép-európai nyári idő Tvrtko
> > Ursulin
> > 
> > wrote:
> >> On 13/07/2026 14:07, Timur Kristóf wrote:
> >>> These were used without ever calling get()/put() on them.
> >> 
> >>> Implement it like on GFX7-8:
> >> Used as in how? Are they even enabled without this change and if not
> >> then does this patch fixes something other than being prep work for soft
> >> reset?
> > 
> > If you open gfx_v6_0.c and search for priv_reg or priv_inst, you can see
> > that the interrupts are used in the same manner as gfx7 and newer, but
> > without get() and put().
> 
> Yes, they are used in code. Are they used in reality was my question. :)

Looking at the code I think the original intention was to wire up these 
interrupts. I think the hardware actually supports these interrupts and the 
author of the code simply forgot to call get() and put().

If you are not convinced, maybe Alex or Christian can confirm whether the 
interrupt actually exists on this HW generation. If it turns out it doesn't 
exist then we should just delete this code.

> I ask because it appears that without amdgpu_irq_get() they may not even
> get enabled so never received. Yes or no? Consequences if yes?

Yes, that sounds correct. The consequence is that the interrupt will now be 
enabled and the HW will tell us about some illegal register access and some 
illegal instructions when it happens.

To give you additional context: for the purpose of diagnosing issues related 
to GPU hangs and recovery, all additional information helps. If the CP has 
interrupts to tell us about some error cases, we should absolutely use that.

Side note: from the register definitions it looks like the HW actually supports 
other interrupts which would be interesting to enable in the future.
In this patch I just wanted to fix up the two that were already in the code.

> 
> >>> * Call amdgpu_irq_get() from gfx_v6_0_late_init()
> >>> * Call amdgpu_irq_put() from gfx_v6_0_hw_fini()
> >>> 
> >>> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> >>> ---
> >>> 
> >>>    drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 19 +++++++++++++++++++
> >>>    1 file changed, 19 insertions(+)
> >>> 
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> >>> b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c index 5b570a4b5c01..1c7cd265fbca
> >>> 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> >>> @@ -3131,6 +3131,22 @@ static int gfx_v6_0_early_init(struct
> >>> amdgpu_ip_block *ip_block)>
> >>> 
> >>>    	return 0;
> >>>    
> >>>    }
> >>> 
> >>> +static int gfx_v6_0_late_init(struct amdgpu_ip_block *ip_block)
> >>> +{
> >>> +	struct amdgpu_device *adev = ip_block->adev;
> >>> +	int r;
> >>> +
> >>> +	r = amdgpu_irq_get(adev, &adev->gfx.priv_reg_irq, 0);
> >>> +	if (r)
> >>> +		return r;
> >>> +
> >>> +	r = amdgpu_irq_get(adev, &adev->gfx.priv_inst_irq, 0);
> >>> +	if (r)
> >>> +		return r;
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>> 
> >>>    static int gfx_v6_0_sw_init(struct amdgpu_ip_block *ip_block)
> >>>    {
> >>>    
> >>>    	struct amdgpu_ring *ring;
> >>> 
> >>> @@ -3243,6 +3259,8 @@ static int gfx_v6_0_hw_fini(struct amdgpu_ip_block
> >>> *ip_block)>
> >>> 
> >>>    {
> >>>    
> >>>    	struct amdgpu_device *adev = ip_block->adev;
> >>> 
> >>> +	amdgpu_irq_put(adev, &adev->gfx.priv_reg_irq, 0);
> >>> +	amdgpu_irq_put(adev, &adev->gfx.priv_inst_irq, 0);
> >>> 
> >>>    	gfx_v6_0_cp_enable(adev, false);
> >>>    	adev->gfx.rlc.funcs->stop(adev);
> >>>    	gfx_v6_0_fini_pg(adev);
> >>> 
> >>> @@ -3532,6 +3550,7 @@ static void gfx_v6_0_emit_mem_sync(struct
> >>> amdgpu_ring *ring)>
> >>> 
> >>>    static const struct amd_ip_funcs gfx_v6_0_ip_funcs = {
> >>>    
> >>>    	.name = "gfx_v6_0",
> >>>    	.early_init = gfx_v6_0_early_init,
> >>> 
> >>> +	.late_init = gfx_v6_0_late_init,
> >>> 
> >>>    	.sw_init = gfx_v6_0_sw_init,
> >>>    	.sw_fini = gfx_v6_0_sw_fini,
> >>>    	.hw_init = gfx_v6_0_hw_init,