Re: [PATCH 6/9] drm/amdgpu: separate IH and IRQ funcs

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

Am 26.09.2018 um 08:05 schrieb Huang Rui:
> On Mon, Sep 24, 2018 at 02:38:17PM +0200, Christian König wrote:
> > One for the ring buffer and one for the IV handling.
> >
> > Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> Reviewed-by: Huang Rui <ray.huang@xxxxxxx>
>
> How about merge amdgpu_ih.c into amdgpu_irq.c? As I think, we don't need
> two common interrupt handle files. IH is actually the hw ip block name, and
> the meaning is actually the same with irq. We can put all common irq handle
> include ih ring init functions into irq.c. If you also agree, I will file
> the patch.

I actually dropped this patch, but we certainly need two different files.

The one is for the IH ring buffer, which Vega10 actually has 3 of.

The other one is for the IRQ handling which does the tracking how often 
interrupt sources are enabled, how to route them etc...

E.g. for that we would still have one instance per device.

Regards,
Christian.

> Thanks,
> Ray
>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h  | 11 ++++-------
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c |  4 ++--
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h | 11 ++++++++++-
> >   drivers/gpu/drm/amd/amdgpu/cik_ih.c     |  8 ++++++--
> >   drivers/gpu/drm/amd/amdgpu/cz_ih.c      |  8 ++++++--
> >   drivers/gpu/drm/amd/amdgpu/iceland_ih.c |  8 ++++++--
> >   drivers/gpu/drm/amd/amdgpu/si_ih.c      |  8 ++++++--
> >   drivers/gpu/drm/amd/amdgpu/tonga_ih.c   |  8 ++++++--
> >   drivers/gpu/drm/amd/amdgpu/vega10_ih.c  |  8 ++++++--
> >   9 files changed, 52 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
> > index 9ce8c93ec19b..d88f82321ee4 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
> > @@ -45,22 +45,19 @@ struct amdgpu_ih_ring {
> >   	bool			use_doorbell;
> >   	bool			use_bus_addr;
> >   	dma_addr_t		rb_dma_addr; /* only used when use_bus_addr = true */
> > +
> > +	const struct amdgpu_ih_funcs	*funcs;
> >   };
> >   
> >   /* provided by the ih block */
> >   struct amdgpu_ih_funcs {
> >   	/* ring read/write ptr handling, called from interrupt context */
> >   	u32 (*get_wptr)(struct amdgpu_device *adev);
> > -	bool (*prescreen_iv)(struct amdgpu_device *adev);
> > -	void (*decode_iv)(struct amdgpu_device *adev,
> > -			  struct amdgpu_iv_entry *entry);
> >   	void (*set_rptr)(struct amdgpu_device *adev);
> >   };
> >   
> > -#define amdgpu_ih_get_wptr(adev) (adev)->irq.ih_funcs->get_wptr((adev))
> > -#define amdgpu_ih_prescreen_iv(adev) (adev)->irq.ih_funcs->prescreen_iv((adev))
> > -#define amdgpu_ih_decode_iv(adev, iv) (adev)->irq.ih_funcs->decode_iv((adev), (iv))
> > -#define amdgpu_ih_set_rptr(adev) (adev)->irq.ih_funcs->set_rptr((adev))
> > +#define amdgpu_ih_get_wptr(adev) (adev)->irq.ih.funcs->get_wptr((adev))
> > +#define amdgpu_ih_set_rptr(adev) (adev)->irq.ih.funcs->set_rptr((adev))
> >   
> >   int amdgpu_ih_ring_init(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih,
> >   			unsigned ring_size, bool use_bus_addr);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> > index 52c17f6219a7..8e5ce25f3fe1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> > @@ -163,14 +163,14 @@ static void amdgpu_irq_callback(struct amdgpu_device *adev,
> >   	struct amdgpu_iv_entry entry;
> >   
> >   	/* Prescreening of high-frequency interrupts */
> > -	if (!amdgpu_ih_prescreen_iv(adev))
> > +	if (!amdgpu_irq_prescreen_iv(adev))
> >   		return;
> >   
> >   	/* Before dispatching irq to IP blocks, send it to amdkfd */
> >   	amdgpu_amdkfd_interrupt(adev, (const void *) &ih->ring[ring_index]);
> >   
> >   	entry.iv_entry = (const uint32_t *)&ih->ring[ring_index];
> > -	amdgpu_ih_decode_iv(adev, &entry);
> > +	amdgpu_irq_decode_iv(adev, &entry);
> >   
> >   	amdgpu_irq_dispatch(adev, &entry);
> >   }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> > index f6ce171cb8aa..3cc0e7ce40a0 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> > @@ -68,6 +68,12 @@ struct amdgpu_irq_client {
> >   	struct amdgpu_irq_src **sources;
> >   };
> >   
> > +struct amdgpu_irq_funcs {
> > +	bool (*prescreen_iv)(struct amdgpu_device *adev);
> > +	void (*decode_iv)(struct amdgpu_device *adev,
> > +			  struct amdgpu_iv_entry *entry);
> > +};
> > +
> >   /* provided by interrupt generating IP blocks */
> >   struct amdgpu_irq_src_funcs {
> >   	int (*set)(struct amdgpu_device *adev, struct amdgpu_irq_src *source,
> > @@ -89,12 +95,12 @@ struct amdgpu_irq {
> >   
> >   	/* interrupt ring */
> >   	struct amdgpu_ih_ring		ih;
> > -	const struct amdgpu_ih_funcs	*ih_funcs;
> >   
> >   	/* gen irq stuff */
> >   	struct irq_domain		*domain; /* GPU irq controller domain */
> >   	unsigned			virq[AMDGPU_MAX_IRQ_SRC_ID];
> >   	uint32_t                        srbm_soft_reset;
> > +	const struct amdgpu_irq_funcs	*funcs;
> >   };
> >   
> >   void amdgpu_irq_disable_all(struct amdgpu_device *adev);
> > @@ -121,4 +127,7 @@ int amdgpu_irq_add_domain(struct amdgpu_device *adev);
> >   void amdgpu_irq_remove_domain(struct amdgpu_device *adev);
> >   unsigned amdgpu_irq_create_mapping(struct amdgpu_device *adev, unsigned src_id);
> >   
> > +#define amdgpu_irq_prescreen_iv(adev) (adev)->irq.funcs->prescreen_iv((adev))
> > +#define amdgpu_irq_decode_iv(adev, iv) (adev)->irq.funcs->decode_iv((adev), (iv))
> > +
> >   #endif
> > diff --git a/drivers/gpu/drm/amd/amdgpu/cik_ih.c b/drivers/gpu/drm/amd/amdgpu/cik_ih.c
> > index b5775c6a857b..161f0225749c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/cik_ih.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/cik_ih.c
> > @@ -461,14 +461,18 @@ static const struct amd_ip_funcs cik_ih_ip_funcs = {
> >   
> >   static const struct amdgpu_ih_funcs cik_ih_funcs = {
> >   	.get_wptr = cik_ih_get_wptr,
> > +	.set_rptr = cik_ih_set_rptr
> > +};
> > +
> > +static const struct amdgpu_irq_funcs cik_irq_funcs = {
> >   	.prescreen_iv = cik_ih_prescreen_iv,
> >   	.decode_iv = cik_ih_decode_iv,
> > -	.set_rptr = cik_ih_set_rptr
> >   };
> >   
> >   static void cik_ih_set_interrupt_funcs(struct amdgpu_device *adev)
> >   {
> > -	adev->irq.ih_funcs = &cik_ih_funcs;
> > +	adev->irq.ih.funcs = &cik_ih_funcs;
> > +	adev->irq.funcs = &cik_irq_funcs;
> >   }
> >   
> >   const struct amdgpu_ip_block_version cik_ih_ip_block =
> > diff --git a/drivers/gpu/drm/amd/amdgpu/cz_ih.c b/drivers/gpu/drm/amd/amdgpu/cz_ih.c
> > index df5ac4d85a00..648ecd774611 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/cz_ih.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/cz_ih.c
> > @@ -442,14 +442,18 @@ static const struct amd_ip_funcs cz_ih_ip_funcs = {
> >   
> >   static const struct amdgpu_ih_funcs cz_ih_funcs = {
> >   	.get_wptr = cz_ih_get_wptr,
> > +	.set_rptr = cz_ih_set_rptr
> > +};
> > +
> > +static const struct amdgpu_irq_funcs cz_irq_funcs = {
> >   	.prescreen_iv = cz_ih_prescreen_iv,
> >   	.decode_iv = cz_ih_decode_iv,
> > -	.set_rptr = cz_ih_set_rptr
> >   };
> >   
> >   static void cz_ih_set_interrupt_funcs(struct amdgpu_device *adev)
> >   {
> > -	adev->irq.ih_funcs = &cz_ih_funcs;
> > +	adev->irq.ih.funcs = &cz_ih_funcs;
> > +	adev->irq.funcs = &cz_irq_funcs;
> >   }
> >   
> >   const struct amdgpu_ip_block_version cz_ih_ip_block =
> > diff --git a/drivers/gpu/drm/amd/amdgpu/iceland_ih.c b/drivers/gpu/drm/amd/amdgpu/iceland_ih.c
> > index cf0fc61aebe6..6139186ccd36 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/iceland_ih.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/iceland_ih.c
> > @@ -440,14 +440,18 @@ static const struct amd_ip_funcs iceland_ih_ip_funcs = {
> >   
> >   static const struct amdgpu_ih_funcs iceland_ih_funcs = {
> >   	.get_wptr = iceland_ih_get_wptr,
> > +	.set_rptr = iceland_ih_set_rptr
> > +};
> > +
> > +static const struct amdgpu_irq_funcs iceland_irq_funcs = {
> >   	.prescreen_iv = iceland_ih_prescreen_iv,
> >   	.decode_iv = iceland_ih_decode_iv,
> > -	.set_rptr = iceland_ih_set_rptr
> >   };
> >   
> >   static void iceland_ih_set_interrupt_funcs(struct amdgpu_device *adev)
> >   {
> > -	adev->irq.ih_funcs = &iceland_ih_funcs;
> > +	adev->irq.ih.funcs = &iceland_ih_funcs;
> > +	adev->irq.funcs = &iceland_irq_funcs;
> >   }
> >   
> >   const struct amdgpu_ip_block_version iceland_ih_ip_block =
> > diff --git a/drivers/gpu/drm/amd/amdgpu/si_ih.c b/drivers/gpu/drm/amd/amdgpu/si_ih.c
> > index b3d7d9f83202..16f212f3b534 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/si_ih.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/si_ih.c
> > @@ -301,14 +301,18 @@ static const struct amd_ip_funcs si_ih_ip_funcs = {
> >   
> >   static const struct amdgpu_ih_funcs si_ih_funcs = {
> >   	.get_wptr = si_ih_get_wptr,
> > +	.set_rptr = si_ih_set_rptr
> > +};
> > +
> > +static const struct amdgpu_irq_funcs si_irq_funcs = {
> >   	.prescreen_iv = si_ih_prescreen_iv,
> >   	.decode_iv = si_ih_decode_iv,
> > -	.set_rptr = si_ih_set_rptr
> >   };
> >   
> >   static void si_ih_set_interrupt_funcs(struct amdgpu_device *adev)
> >   {
> > -	adev->irq.ih_funcs = &si_ih_funcs;
> > +	adev->irq.ih.funcs = &si_ih_funcs;
> > +	adev->irq.funcs = &si_irq_funcs;
> >   }
> >   
> >   const struct amdgpu_ip_block_version si_ih_ip_block =
> > diff --git a/drivers/gpu/drm/amd/amdgpu/tonga_ih.c b/drivers/gpu/drm/amd/amdgpu/tonga_ih.c
> > index 3abffd06b5c7..6120ac3c7667 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/tonga_ih.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/tonga_ih.c
> > @@ -506,14 +506,18 @@ static const struct amd_ip_funcs tonga_ih_ip_funcs = {
> >   
> >   static const struct amdgpu_ih_funcs tonga_ih_funcs = {
> >   	.get_wptr = tonga_ih_get_wptr,
> > +	.set_rptr = tonga_ih_set_rptr
> > +};
> > +
> > +static const struct amdgpu_irq_funcs tonga_irq_funcs = {
> >   	.prescreen_iv = tonga_ih_prescreen_iv,
> >   	.decode_iv = tonga_ih_decode_iv,
> > -	.set_rptr = tonga_ih_set_rptr
> >   };
> >   
> >   static void tonga_ih_set_interrupt_funcs(struct amdgpu_device *adev)
> >   {
> > -	adev->irq.ih_funcs = &tonga_ih_funcs;
> > +	adev->irq.ih.funcs = &tonga_ih_funcs;
> > +	adev->irq.funcs = &tonga_irq_funcs;
> >   }
> >   
> >   const struct amdgpu_ip_block_version tonga_ih_ip_block =
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> > index a99f71797aa3..a9737adf8392 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> > @@ -487,14 +487,18 @@ const struct amd_ip_funcs vega10_ih_ip_funcs = {
> >   
> >   static const struct amdgpu_ih_funcs vega10_ih_funcs = {
> >   	.get_wptr = vega10_ih_get_wptr,
> > +	.set_rptr = vega10_ih_set_rptr
> > +};
> > +
> > +static const struct amdgpu_irq_funcs vega10_irq_funcs = {
> >   	.prescreen_iv = vega10_ih_prescreen_iv,
> >   	.decode_iv = vega10_ih_decode_iv,
> > -	.set_rptr = vega10_ih_set_rptr
> >   };
> >   
> >   static void vega10_ih_set_interrupt_funcs(struct amdgpu_device *adev)
> >   {
> > -	adev->irq.ih_funcs = &vega10_ih_funcs;
> > +	adev->irq.ih.funcs = &vega10_ih_funcs;
> > +	adev->irq.funcs = &vega10_irq_funcs;
> >   }
> >   
> >   const struct amdgpu_ip_block_version vega10_ih_ip_block =
> > --
> > 2.14.1
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx