> -----Original Message----- > From: Christian König [mailto:ckoenig.leichtzumerken@xxxxxxxxx] > Sent: Wednesday, September 26, 2018 4:09 PM > To: Huang, Ray <Ray.Huang@xxxxxxx> > Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH 6/9] drm/amdgpu: separate IH and IRQ funcs > > 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. > So you want to separate the hardware-layer abstraction function and IRQ handling for upper-layer on different file? That's also fine. Thanks, Ray > 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
