* [PATCH] drm/amdgpu: add ih call to process until checkpoint @ 2021-02-23 21:10 Jonathan Kim 2021-02-23 22:45 ` Andrey Grodzovsky 2021-02-24 9:16 ` Christian König 0 siblings, 2 replies; 10+ messages in thread From: Jonathan Kim @ 2021-02-23 21:10 UTC (permalink / raw) To: amd-gfx; +Cc: Philip.Yang, Felix.Kuehling, Jonathan Kim Add IH function to allow caller to process ring entries until the checkpoint write pointer. Suggested-by: Felix Kuehling <felix.kuehling@amd.com> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 46 +++++++++++++++++++++++++- drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 2 ++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c index dc852af4f3b7..cae50af9559d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c @@ -22,7 +22,7 @@ */ #include <linux/dma-mapping.h> - +#include <linux/processor.h> #include "amdgpu.h" #include "amdgpu_ih.h" @@ -160,6 +160,50 @@ void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const uint32_t *iv, } } +/** + * amdgpu_ih_wait_on_checkpoint_process - wait to process IVs up to checkpoint + * + * @adev: amdgpu_device pointer + * @ih: ih ring to process + * + * Used to ensure ring has processed IVs up to the checkpoint write pointer. + */ +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device *adev, + struct amdgpu_ih_ring *ih) +{ + u32 prev_rptr, cur_rptr, checkpoint_wptr; + + if (!ih->enabled || adev->shutdown) + return -ENODEV; + + cur_rptr = READ_ONCE(ih->rptr); + /* Order read of current rptr with checktpoint wptr. */ + mb(); + checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); + + /* allow rptr to wrap around */ + if (cur_rptr > checkpoint_wptr) { + spin_begin(); + do { + spin_cpu_relax(); + prev_rptr = cur_rptr; + cur_rptr = READ_ONCE(ih->rptr); + } while (cur_rptr >= prev_rptr); + spin_end(); + } + + /* wait for rptr to catch up to or pass checkpoint. */ + spin_begin(); + do { + spin_cpu_relax(); + prev_rptr = cur_rptr; + cur_rptr = READ_ONCE(ih->rptr); + } while (cur_rptr >= prev_rptr && cur_rptr < checkpoint_wptr); + spin_end(); + + return 0; +} + /** * amdgpu_ih_process - interrupt handler * diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h index 6ed4a85fc7c3..6817f0a812d2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h @@ -87,6 +87,8 @@ int amdgpu_ih_ring_init(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih, void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih); void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const uint32_t *iv, unsigned int num_dw); +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device *adev, + struct amdgpu_ih_ring *ih); int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih); void amdgpu_ih_decode_iv_helper(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih, -- 2.17.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: add ih call to process until checkpoint 2021-02-23 21:10 [PATCH] drm/amdgpu: add ih call to process until checkpoint Jonathan Kim @ 2021-02-23 22:45 ` Andrey Grodzovsky 2021-02-24 9:16 ` Christian König 1 sibling, 0 replies; 10+ messages in thread From: Andrey Grodzovsky @ 2021-02-23 22:45 UTC (permalink / raw) To: Jonathan Kim, amd-gfx; +Cc: Philip.Yang, Felix.Kuehling On 2021-02-23 4:10 p.m., Jonathan Kim wrote: > Add IH function to allow caller to process ring entries until the > checkpoint write pointer. > > Suggested-by: Felix Kuehling <felix.kuehling@amd.com> > Signed-off-by: Jonathan Kim <jonathan.kim@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 46 +++++++++++++++++++++++++- > drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 2 ++ > 2 files changed, 47 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c > index dc852af4f3b7..cae50af9559d 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c > @@ -22,7 +22,7 @@ > */ > > #include <linux/dma-mapping.h> > - > +#include <linux/processor.h> > #include "amdgpu.h" > #include "amdgpu_ih.h" > > @@ -160,6 +160,50 @@ void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const uint32_t *iv, > } > } > > +/** > + * amdgpu_ih_wait_on_checkpoint_process - wait to process IVs up to checkpoint > + * > + * @adev: amdgpu_device pointer > + * @ih: ih ring to process > + * > + * Used to ensure ring has processed IVs up to the checkpoint write pointer. > + */ > +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device *adev, > + struct amdgpu_ih_ring *ih) > +{ > + u32 prev_rptr, cur_rptr, checkpoint_wptr; > + > + if (!ih->enabled || adev->shutdown) > + return -ENODEV; > + > + cur_rptr = READ_ONCE(ih->rptr); > + /* Order read of current rptr with checktpoint wptr. */ > + mb(); > + checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); > + Isn't rmb() enough in this case ? If I understand correctly you just want to confirm that you are not reading stale checkpoint_wptr while reading fresh cur_rptr. Andrey > + /* allow rptr to wrap around */ > + if (cur_rptr > checkpoint_wptr) { > + spin_begin(); > + do { > + spin_cpu_relax(); > + prev_rptr = cur_rptr; > + cur_rptr = READ_ONCE(ih->rptr); > + } while (cur_rptr >= prev_rptr); > + spin_end(); > + } > + > + /* wait for rptr to catch up to or pass checkpoint. */ > + spin_begin(); > + do { > + spin_cpu_relax(); > + prev_rptr = cur_rptr; > + cur_rptr = READ_ONCE(ih->rptr); > + } while (cur_rptr >= prev_rptr && cur_rptr < checkpoint_wptr); > + spin_end(); > + > + return 0; > +} > + > /** > * amdgpu_ih_process - interrupt handler > * > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h > index 6ed4a85fc7c3..6817f0a812d2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h > @@ -87,6 +87,8 @@ int amdgpu_ih_ring_init(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih, > void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih); > void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const uint32_t *iv, > unsigned int num_dw); > +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device *adev, > + struct amdgpu_ih_ring *ih); > int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih); > void amdgpu_ih_decode_iv_helper(struct amdgpu_device *adev, > struct amdgpu_ih_ring *ih, _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: add ih call to process until checkpoint 2021-02-23 21:10 [PATCH] drm/amdgpu: add ih call to process until checkpoint Jonathan Kim 2021-02-23 22:45 ` Andrey Grodzovsky @ 2021-02-24 9:16 ` Christian König 2021-02-24 15:54 ` Kim, Jonathan 1 sibling, 1 reply; 10+ messages in thread From: Christian König @ 2021-02-24 9:16 UTC (permalink / raw) To: Jonathan Kim, amd-gfx; +Cc: Philip.Yang, Felix.Kuehling Am 23.02.21 um 22:10 schrieb Jonathan Kim: > Add IH function to allow caller to process ring entries until the > checkpoint write pointer. This needs a better description of what this will be used for. > > Suggested-by: Felix Kuehling <felix.kuehling@amd.com> > Signed-off-by: Jonathan Kim <jonathan.kim@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 46 +++++++++++++++++++++++++- > drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 2 ++ > 2 files changed, 47 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c > index dc852af4f3b7..cae50af9559d 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c > @@ -22,7 +22,7 @@ > */ > > #include <linux/dma-mapping.h> > - > +#include <linux/processor.h> > #include "amdgpu.h" > #include "amdgpu_ih.h" > > @@ -160,6 +160,50 @@ void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const uint32_t *iv, > } > } > > +/** > + * amdgpu_ih_wait_on_checkpoint_process - wait to process IVs up to checkpoint > + * > + * @adev: amdgpu_device pointer > + * @ih: ih ring to process > + * > + * Used to ensure ring has processed IVs up to the checkpoint write pointer. > + */ > +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device *adev, > + struct amdgpu_ih_ring *ih) > +{ > + u32 prev_rptr, cur_rptr, checkpoint_wptr; > + > + if (!ih->enabled || adev->shutdown) > + return -ENODEV; > + > + cur_rptr = READ_ONCE(ih->rptr); > + /* Order read of current rptr with checktpoint wptr. */ > + mb(); > + checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); > + > + /* allow rptr to wrap around */ > + if (cur_rptr > checkpoint_wptr) { > + spin_begin(); > + do { > + spin_cpu_relax(); > + prev_rptr = cur_rptr; > + cur_rptr = READ_ONCE(ih->rptr); > + } while (cur_rptr >= prev_rptr); > + spin_end(); That's a certain NAK since it busy waits for IH processing. We need some event to trigger here. > + } > + > + /* wait for rptr to catch up to or pass checkpoint. */ > + spin_begin(); > + do { > + spin_cpu_relax(); > + prev_rptr = cur_rptr; > + cur_rptr = READ_ONCE(ih->rptr); > + } while (cur_rptr >= prev_rptr && cur_rptr < checkpoint_wptr); Same of course here. Christian. > + spin_end(); > + > + return 0; > +} > + > /** > * amdgpu_ih_process - interrupt handler > * > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h > index 6ed4a85fc7c3..6817f0a812d2 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h > @@ -87,6 +87,8 @@ int amdgpu_ih_ring_init(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih, > void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih); > void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const uint32_t *iv, > unsigned int num_dw); > +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device *adev, > + struct amdgpu_ih_ring *ih); > int amdgpu_ih_process(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih); > void amdgpu_ih_decode_iv_helper(struct amdgpu_device *adev, > struct amdgpu_ih_ring *ih, _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: add ih call to process until checkpoint 2021-02-24 9:16 ` Christian König @ 2021-02-24 15:54 ` Kim, Jonathan 2021-02-25 3:15 ` Felix Kuehling 0 siblings, 1 reply; 10+ messages in thread From: Kim, Jonathan @ 2021-02-24 15:54 UTC (permalink / raw) To: Koenig, Christian, amd-gfx; +Cc: Yang, Philip, Kuehling, Felix [AMD Official Use Only - Internal Distribution Only] > -----Original Message----- > From: Koenig, Christian <Christian.Koenig@amd.com> > Sent: Wednesday, February 24, 2021 4:17 AM > To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd- > gfx@lists.freedesktop.org > Cc: Yang, Philip <Philip.Yang@amd.com>; Kuehling, Felix > <Felix.Kuehling@amd.com> > Subject: Re: [PATCH] drm/amdgpu: add ih call to process until checkpoint > > Am 23.02.21 um 22:10 schrieb Jonathan Kim: > > Add IH function to allow caller to process ring entries until the > > checkpoint write pointer. > > This needs a better description of what this will be used for. Felix or Philip could elaborate better for HMM needs. Debugging tools requires this but it's in experimental mode at the moment so probably not the best place to describe here. > > > > > Suggested-by: Felix Kuehling <felix.kuehling@amd.com> > > Signed-off-by: Jonathan Kim <jonathan.kim@amd.com> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 46 > +++++++++++++++++++++++++- > > drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 2 ++ > > 2 files changed, 47 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c > > index dc852af4f3b7..cae50af9559d 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c > > @@ -22,7 +22,7 @@ > > */ > > > > #include <linux/dma-mapping.h> > > - > > +#include <linux/processor.h> > > #include "amdgpu.h" > > #include "amdgpu_ih.h" > > > > @@ -160,6 +160,50 @@ void amdgpu_ih_ring_write(struct > amdgpu_ih_ring *ih, const uint32_t *iv, > > } > > } > > > > +/** > > + * amdgpu_ih_wait_on_checkpoint_process - wait to process IVs up to > > +checkpoint > > + * > > + * @adev: amdgpu_device pointer > > + * @ih: ih ring to process > > + * > > + * Used to ensure ring has processed IVs up to the checkpoint write > pointer. > > + */ > > +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device > *adev, > > +struct amdgpu_ih_ring *ih) > > +{ > > +u32 prev_rptr, cur_rptr, checkpoint_wptr; > > + > > +if (!ih->enabled || adev->shutdown) > > +return -ENODEV; > > + > > +cur_rptr = READ_ONCE(ih->rptr); > > +/* Order read of current rptr with checktpoint wptr. */ > > +mb(); > > +checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); > > + > > +/* allow rptr to wrap around */ > > +if (cur_rptr > checkpoint_wptr) { > > +spin_begin(); > > +do { > > +spin_cpu_relax(); > > +prev_rptr = cur_rptr; > > +cur_rptr = READ_ONCE(ih->rptr); > > +} while (cur_rptr >= prev_rptr); > > +spin_end(); > > That's a certain NAK since it busy waits for IH processing. We need some > event to trigger here. The function is meant to be just a waiter up to the checkpoint. There's a need to guarantee that "stale" interrupts have been processed on check before doing other stuff after call. The description could be improved to clarify that. Would busy waiting only on a locked ring help? I assume an unlocked ring means nothing to process so no need to wait and we can exit early. Or is it better to just to process the entries up to the checkpoint (maybe adjust amdgpu_ih_process for this need like adding a bool arg to skip restart or something)? Thanks, Jon > > > +} > > + > > +/* wait for rptr to catch up to or pass checkpoint. */ > > +spin_begin(); > > +do { > > +spin_cpu_relax(); > > +prev_rptr = cur_rptr; > > +cur_rptr = READ_ONCE(ih->rptr); > > +} while (cur_rptr >= prev_rptr && cur_rptr < checkpoint_wptr); > > Same of course here. > > Christian. > > > +spin_end(); > > + > > +return 0; > > +} > > + > > /** > > * amdgpu_ih_process - interrupt handler > > * > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h > > index 6ed4a85fc7c3..6817f0a812d2 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h > > @@ -87,6 +87,8 @@ int amdgpu_ih_ring_init(struct amdgpu_device *adev, > struct amdgpu_ih_ring *ih, > > void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct > amdgpu_ih_ring *ih); > > void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const uint32_t *iv, > > unsigned int num_dw); > > +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device > *adev, > > +struct amdgpu_ih_ring *ih); > > int amdgpu_ih_process(struct amdgpu_device *adev, struct > amdgpu_ih_ring *ih); > > void amdgpu_ih_decode_iv_helper(struct amdgpu_device *adev, > > struct amdgpu_ih_ring *ih, _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: add ih call to process until checkpoint 2021-02-24 15:54 ` Kim, Jonathan @ 2021-02-25 3:15 ` Felix Kuehling 2021-02-25 13:53 ` Christian König 0 siblings, 1 reply; 10+ messages in thread From: Felix Kuehling @ 2021-02-25 3:15 UTC (permalink / raw) To: Kim, Jonathan, Koenig, Christian, amd-gfx; +Cc: Yang, Philip On 2021-02-24 10:54 a.m., Kim, Jonathan wrote: > [AMD Official Use Only - Internal Distribution Only] > >> -----Original Message----- >> From: Koenig, Christian <Christian.Koenig@amd.com> >> Sent: Wednesday, February 24, 2021 4:17 AM >> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd- >> gfx@lists.freedesktop.org >> Cc: Yang, Philip <Philip.Yang@amd.com>; Kuehling, Felix >> <Felix.Kuehling@amd.com> >> Subject: Re: [PATCH] drm/amdgpu: add ih call to process until checkpoint >> >> Am 23.02.21 um 22:10 schrieb Jonathan Kim: >>> Add IH function to allow caller to process ring entries until the >>> checkpoint write pointer. >> This needs a better description of what this will be used for. > Felix or Philip could elaborate better for HMM needs. > Debugging tools requires this but it's in experimental mode at the moment so probably not the best place to describe here. On the HMM side we're planning to use this to drain pending page fault interrupts before we unmap memory. That should address phantom VM faults after memory is unmapped. Regards, Felix > >>> Suggested-by: Felix Kuehling <felix.kuehling@amd.com> >>> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 46 >> +++++++++++++++++++++++++- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 2 ++ >>> 2 files changed, 47 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>> index dc852af4f3b7..cae50af9559d 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>> @@ -22,7 +22,7 @@ >>> */ >>> >>> #include <linux/dma-mapping.h> >>> - >>> +#include <linux/processor.h> >>> #include "amdgpu.h" >>> #include "amdgpu_ih.h" >>> >>> @@ -160,6 +160,50 @@ void amdgpu_ih_ring_write(struct >> amdgpu_ih_ring *ih, const uint32_t *iv, >>> } >>> } >>> >>> +/** >>> + * amdgpu_ih_wait_on_checkpoint_process - wait to process IVs up to >>> +checkpoint >>> + * >>> + * @adev: amdgpu_device pointer >>> + * @ih: ih ring to process >>> + * >>> + * Used to ensure ring has processed IVs up to the checkpoint write >> pointer. >>> + */ >>> +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device >> *adev, >>> +struct amdgpu_ih_ring *ih) >>> +{ >>> +u32 prev_rptr, cur_rptr, checkpoint_wptr; >>> + >>> +if (!ih->enabled || adev->shutdown) >>> +return -ENODEV; >>> + >>> +cur_rptr = READ_ONCE(ih->rptr); >>> +/* Order read of current rptr with checktpoint wptr. */ >>> +mb(); >>> +checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); >>> + >>> +/* allow rptr to wrap around */ >>> +if (cur_rptr > checkpoint_wptr) { >>> +spin_begin(); >>> +do { >>> +spin_cpu_relax(); >>> +prev_rptr = cur_rptr; >>> +cur_rptr = READ_ONCE(ih->rptr); >>> +} while (cur_rptr >= prev_rptr); >>> +spin_end(); >> That's a certain NAK since it busy waits for IH processing. We need some >> event to trigger here. > The function is meant to be just a waiter up to the checkpoint. > There's a need to guarantee that "stale" interrupts have been processed on check before doing other stuff after call. > The description could be improved to clarify that. > > Would busy waiting only on a locked ring help? I assume an unlocked ring means nothing to process so no need to wait and we can exit early. Or is it better to just to process the entries up to the checkpoint (maybe adjust amdgpu_ih_process for this need like adding a bool arg to skip restart or something)? > > Thanks, > > Jon > >>> +} >>> + >>> +/* wait for rptr to catch up to or pass checkpoint. */ >>> +spin_begin(); >>> +do { >>> +spin_cpu_relax(); >>> +prev_rptr = cur_rptr; >>> +cur_rptr = READ_ONCE(ih->rptr); >>> +} while (cur_rptr >= prev_rptr && cur_rptr < checkpoint_wptr); >> Same of course here. >> >> Christian. >> >>> +spin_end(); >>> + >>> +return 0; >>> +} >>> + >>> /** >>> * amdgpu_ih_process - interrupt handler >>> * >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>> index 6ed4a85fc7c3..6817f0a812d2 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>> @@ -87,6 +87,8 @@ int amdgpu_ih_ring_init(struct amdgpu_device *adev, >> struct amdgpu_ih_ring *ih, >>> void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct >> amdgpu_ih_ring *ih); >>> void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const uint32_t *iv, >>> unsigned int num_dw); >>> +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device >> *adev, >>> +struct amdgpu_ih_ring *ih); >>> int amdgpu_ih_process(struct amdgpu_device *adev, struct >> amdgpu_ih_ring *ih); >>> void amdgpu_ih_decode_iv_helper(struct amdgpu_device *adev, >>> struct amdgpu_ih_ring *ih, _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: add ih call to process until checkpoint 2021-02-25 3:15 ` Felix Kuehling @ 2021-02-25 13:53 ` Christian König 2021-02-25 15:35 ` Felix Kuehling 0 siblings, 1 reply; 10+ messages in thread From: Christian König @ 2021-02-25 13:53 UTC (permalink / raw) To: Felix Kuehling, Kim, Jonathan, Koenig, Christian, amd-gfx; +Cc: Yang, Philip Am 25.02.21 um 04:15 schrieb Felix Kuehling: > On 2021-02-24 10:54 a.m., Kim, Jonathan wrote: >> [AMD Official Use Only - Internal Distribution Only] >> >>> -----Original Message----- >>> From: Koenig, Christian <Christian.Koenig@amd.com> >>> Sent: Wednesday, February 24, 2021 4:17 AM >>> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd- >>> gfx@lists.freedesktop.org >>> Cc: Yang, Philip <Philip.Yang@amd.com>; Kuehling, Felix >>> <Felix.Kuehling@amd.com> >>> Subject: Re: [PATCH] drm/amdgpu: add ih call to process until >>> checkpoint >>> >>> Am 23.02.21 um 22:10 schrieb Jonathan Kim: >>>> Add IH function to allow caller to process ring entries until the >>>> checkpoint write pointer. >>> This needs a better description of what this will be used for. >> Felix or Philip could elaborate better for HMM needs. >> Debugging tools requires this but it's in experimental mode at the >> moment so probably not the best place to describe here. > > On the HMM side we're planning to use this to drain pending page fault > interrupts before we unmap memory. That should address phantom VM > faults after memory is unmapped. Thought so. I suggest to use a wait_event() here which on the waiter side checks ih->lock and add a wake_up_all() at the end of amdgpu_ih_process. I won't touch rptr or wptr at all for this. Regards, Christian. > > Regards, > Felix > > >> >>>> Suggested-by: Felix Kuehling <felix.kuehling@amd.com> >>>> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com> >>>> --- >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 46 >>> +++++++++++++++++++++++++- >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 2 ++ >>>> 2 files changed, 47 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>> index dc852af4f3b7..cae50af9559d 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>> @@ -22,7 +22,7 @@ >>>> */ >>>> >>>> #include <linux/dma-mapping.h> >>>> - >>>> +#include <linux/processor.h> >>>> #include "amdgpu.h" >>>> #include "amdgpu_ih.h" >>>> >>>> @@ -160,6 +160,50 @@ void amdgpu_ih_ring_write(struct >>> amdgpu_ih_ring *ih, const uint32_t *iv, >>>> } >>>> } >>>> >>>> +/** >>>> + * amdgpu_ih_wait_on_checkpoint_process - wait to process IVs up to >>>> +checkpoint >>>> + * >>>> + * @adev: amdgpu_device pointer >>>> + * @ih: ih ring to process >>>> + * >>>> + * Used to ensure ring has processed IVs up to the checkpoint write >>> pointer. >>>> + */ >>>> +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device >>> *adev, >>>> +struct amdgpu_ih_ring *ih) >>>> +{ >>>> +u32 prev_rptr, cur_rptr, checkpoint_wptr; >>>> + >>>> +if (!ih->enabled || adev->shutdown) >>>> +return -ENODEV; >>>> + >>>> +cur_rptr = READ_ONCE(ih->rptr); >>>> +/* Order read of current rptr with checktpoint wptr. */ >>>> +mb(); >>>> +checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); >>>> + >>>> +/* allow rptr to wrap around */ >>>> +if (cur_rptr > checkpoint_wptr) { >>>> +spin_begin(); >>>> +do { >>>> +spin_cpu_relax(); >>>> +prev_rptr = cur_rptr; >>>> +cur_rptr = READ_ONCE(ih->rptr); >>>> +} while (cur_rptr >= prev_rptr); >>>> +spin_end(); >>> That's a certain NAK since it busy waits for IH processing. We need >>> some >>> event to trigger here. >> The function is meant to be just a waiter up to the checkpoint. >> There's a need to guarantee that "stale" interrupts have been >> processed on check before doing other stuff after call. >> The description could be improved to clarify that. >> >> Would busy waiting only on a locked ring help? I assume an unlocked >> ring means nothing to process so no need to wait and we can exit >> early. Or is it better to just to process the entries up to the >> checkpoint (maybe adjust amdgpu_ih_process for this need like adding >> a bool arg to skip restart or something)? >> >> Thanks, >> >> Jon >> >>>> +} >>>> + >>>> +/* wait for rptr to catch up to or pass checkpoint. */ >>>> +spin_begin(); >>>> +do { >>>> +spin_cpu_relax(); >>>> +prev_rptr = cur_rptr; >>>> +cur_rptr = READ_ONCE(ih->rptr); >>>> +} while (cur_rptr >= prev_rptr && cur_rptr < checkpoint_wptr); >>> Same of course here. >>> >>> Christian. >>> >>>> +spin_end(); >>>> + >>>> +return 0; >>>> +} >>>> + >>>> /** >>>> * amdgpu_ih_process - interrupt handler >>>> * >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>> index 6ed4a85fc7c3..6817f0a812d2 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>> @@ -87,6 +87,8 @@ int amdgpu_ih_ring_init(struct amdgpu_device *adev, >>> struct amdgpu_ih_ring *ih, >>>> void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct >>> amdgpu_ih_ring *ih); >>>> void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const >>>> uint32_t *iv, >>>> unsigned int num_dw); >>>> +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device >>> *adev, >>>> +struct amdgpu_ih_ring *ih); >>>> int amdgpu_ih_process(struct amdgpu_device *adev, struct >>> amdgpu_ih_ring *ih); >>>> void amdgpu_ih_decode_iv_helper(struct amdgpu_device *adev, >>>> struct amdgpu_ih_ring *ih, > _______________________________________________ > 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] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: add ih call to process until checkpoint 2021-02-25 13:53 ` Christian König @ 2021-02-25 15:35 ` Felix Kuehling 2021-02-25 16:48 ` Christian König 0 siblings, 1 reply; 10+ messages in thread From: Felix Kuehling @ 2021-02-25 15:35 UTC (permalink / raw) To: Christian König, Kim, Jonathan, Koenig, Christian, amd-gfx Cc: Yang, Philip Am 2021-02-25 um 8:53 a.m. schrieb Christian König: > > > Am 25.02.21 um 04:15 schrieb Felix Kuehling: >> On 2021-02-24 10:54 a.m., Kim, Jonathan wrote: >>> [AMD Official Use Only - Internal Distribution Only] >>> >>>> -----Original Message----- >>>> From: Koenig, Christian <Christian.Koenig@amd.com> >>>> Sent: Wednesday, February 24, 2021 4:17 AM >>>> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd- >>>> gfx@lists.freedesktop.org >>>> Cc: Yang, Philip <Philip.Yang@amd.com>; Kuehling, Felix >>>> <Felix.Kuehling@amd.com> >>>> Subject: Re: [PATCH] drm/amdgpu: add ih call to process until >>>> checkpoint >>>> >>>> Am 23.02.21 um 22:10 schrieb Jonathan Kim: >>>>> Add IH function to allow caller to process ring entries until the >>>>> checkpoint write pointer. >>>> This needs a better description of what this will be used for. >>> Felix or Philip could elaborate better for HMM needs. >>> Debugging tools requires this but it's in experimental mode at the >>> moment so probably not the best place to describe here. >> >> On the HMM side we're planning to use this to drain pending page >> fault interrupts before we unmap memory. That should address phantom >> VM faults after memory is unmapped. > > Thought so. I suggest to use a wait_event() here which on the waiter > side checks ih->lock and add a wake_up_all() at the end of > amdgpu_ih_process. Right. I thought about that and it should be easy to add. The reason to suggest busy waiting first is, that interrupt processing is supposed to be fast. The IRQ handler itself doesn't sleep. So I'd expect the wait time to be short enough that sleeping and scheduling is not worth it. > I won't touch rptr or wptr at all for this. Not sure what's your idea here, using ih->lock. Is it to completely drain all IRQs until the IH ring is completely empty? That can live-lock, if the GPU produces IRQs faster than the kernel can process them. Therefore I was looking at rptr and wptr to drain only IRQs that were already in the queue when the drain call was made. That also ensures that the wait time is bounded and should be short (unless the ring size is huge). Regards, Felix > > Regards, > Christian. > >> >> Regards, >> Felix >> >> >>> >>>>> Suggested-by: Felix Kuehling <felix.kuehling@amd.com> >>>>> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com> >>>>> --- >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 46 >>>> +++++++++++++++++++++++++- >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 2 ++ >>>>> 2 files changed, 47 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>> index dc852af4f3b7..cae50af9559d 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>> @@ -22,7 +22,7 @@ >>>>> */ >>>>> >>>>> #include <linux/dma-mapping.h> >>>>> - >>>>> +#include <linux/processor.h> >>>>> #include "amdgpu.h" >>>>> #include "amdgpu_ih.h" >>>>> >>>>> @@ -160,6 +160,50 @@ void amdgpu_ih_ring_write(struct >>>> amdgpu_ih_ring *ih, const uint32_t *iv, >>>>> } >>>>> } >>>>> >>>>> +/** >>>>> + * amdgpu_ih_wait_on_checkpoint_process - wait to process IVs up to >>>>> +checkpoint >>>>> + * >>>>> + * @adev: amdgpu_device pointer >>>>> + * @ih: ih ring to process >>>>> + * >>>>> + * Used to ensure ring has processed IVs up to the checkpoint write >>>> pointer. >>>>> + */ >>>>> +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device >>>> *adev, >>>>> +struct amdgpu_ih_ring *ih) >>>>> +{ >>>>> +u32 prev_rptr, cur_rptr, checkpoint_wptr; >>>>> + >>>>> +if (!ih->enabled || adev->shutdown) >>>>> +return -ENODEV; >>>>> + >>>>> +cur_rptr = READ_ONCE(ih->rptr); >>>>> +/* Order read of current rptr with checktpoint wptr. */ >>>>> +mb(); >>>>> +checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); >>>>> + >>>>> +/* allow rptr to wrap around */ >>>>> +if (cur_rptr > checkpoint_wptr) { >>>>> +spin_begin(); >>>>> +do { >>>>> +spin_cpu_relax(); >>>>> +prev_rptr = cur_rptr; >>>>> +cur_rptr = READ_ONCE(ih->rptr); >>>>> +} while (cur_rptr >= prev_rptr); >>>>> +spin_end(); >>>> That's a certain NAK since it busy waits for IH processing. We need >>>> some >>>> event to trigger here. >>> The function is meant to be just a waiter up to the checkpoint. >>> There's a need to guarantee that "stale" interrupts have been >>> processed on check before doing other stuff after call. >>> The description could be improved to clarify that. >>> >>> Would busy waiting only on a locked ring help? I assume an unlocked >>> ring means nothing to process so no need to wait and we can exit >>> early. Or is it better to just to process the entries up to the >>> checkpoint (maybe adjust amdgpu_ih_process for this need like adding >>> a bool arg to skip restart or something)? >>> >>> Thanks, >>> >>> Jon >>> >>>>> +} >>>>> + >>>>> +/* wait for rptr to catch up to or pass checkpoint. */ >>>>> +spin_begin(); >>>>> +do { >>>>> +spin_cpu_relax(); >>>>> +prev_rptr = cur_rptr; >>>>> +cur_rptr = READ_ONCE(ih->rptr); >>>>> +} while (cur_rptr >= prev_rptr && cur_rptr < checkpoint_wptr); >>>> Same of course here. >>>> >>>> Christian. >>>> >>>>> +spin_end(); >>>>> + >>>>> +return 0; >>>>> +} >>>>> + >>>>> /** >>>>> * amdgpu_ih_process - interrupt handler >>>>> * >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>> index 6ed4a85fc7c3..6817f0a812d2 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>> @@ -87,6 +87,8 @@ int amdgpu_ih_ring_init(struct amdgpu_device *adev, >>>> struct amdgpu_ih_ring *ih, >>>>> void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct >>>> amdgpu_ih_ring *ih); >>>>> void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const >>>>> uint32_t *iv, >>>>> unsigned int num_dw); >>>>> +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device >>>> *adev, >>>>> +struct amdgpu_ih_ring *ih); >>>>> int amdgpu_ih_process(struct amdgpu_device *adev, struct >>>> amdgpu_ih_ring *ih); >>>>> void amdgpu_ih_decode_iv_helper(struct amdgpu_device *adev, >>>>> struct amdgpu_ih_ring *ih, >> _______________________________________________ >> amd-gfx mailing list >> amd-gfx@lists.freedesktop.org >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cfelix.kuehling%40amd.com%7C84d85e54bdcb4593e07f08d8d994be77%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637498580167313193%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=RvRHB9l4O%2BpbpogUFKUmnMGkqKnecwQCYRHrkxICDqU%3D&reserved=0 >> > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: add ih call to process until checkpoint 2021-02-25 15:35 ` Felix Kuehling @ 2021-02-25 16:48 ` Christian König 2021-02-25 18:33 ` Felix Kuehling 0 siblings, 1 reply; 10+ messages in thread From: Christian König @ 2021-02-25 16:48 UTC (permalink / raw) To: Felix Kuehling, Christian König, Kim, Jonathan, amd-gfx; +Cc: Yang, Philip Am 25.02.21 um 16:35 schrieb Felix Kuehling: > Am 2021-02-25 um 8:53 a.m. schrieb Christian König: >> >> Am 25.02.21 um 04:15 schrieb Felix Kuehling: >>> On 2021-02-24 10:54 a.m., Kim, Jonathan wrote: >>>> [AMD Official Use Only - Internal Distribution Only] >>>> >>>>> -----Original Message----- >>>>> From: Koenig, Christian <Christian.Koenig@amd.com> >>>>> Sent: Wednesday, February 24, 2021 4:17 AM >>>>> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd- >>>>> gfx@lists.freedesktop.org >>>>> Cc: Yang, Philip <Philip.Yang@amd.com>; Kuehling, Felix >>>>> <Felix.Kuehling@amd.com> >>>>> Subject: Re: [PATCH] drm/amdgpu: add ih call to process until >>>>> checkpoint >>>>> >>>>> Am 23.02.21 um 22:10 schrieb Jonathan Kim: >>>>>> Add IH function to allow caller to process ring entries until the >>>>>> checkpoint write pointer. >>>>> This needs a better description of what this will be used for. >>>> Felix or Philip could elaborate better for HMM needs. >>>> Debugging tools requires this but it's in experimental mode at the >>>> moment so probably not the best place to describe here. >>> On the HMM side we're planning to use this to drain pending page >>> fault interrupts before we unmap memory. That should address phantom >>> VM faults after memory is unmapped. >> Thought so. I suggest to use a wait_event() here which on the waiter >> side checks ih->lock and add a wake_up_all() at the end of >> amdgpu_ih_process. > Right. I thought about that and it should be easy to add. The reason to > suggest busy waiting first is, that interrupt processing is supposed to > be fast. The IRQ handler itself doesn't sleep. So I'd expect the wait > time to be short enough that sleeping and scheduling is not worth it. Well the page fault IRQs are processed in a work item, so we busy wait for another thread here and not interrupt context. This in turn can lead to starvation of the work handler and so a life lock as well. > > >> I won't touch rptr or wptr at all for this. > Not sure what's your idea here, using ih->lock. Is it to completely > drain all IRQs until the IH ring is completely empty? Correct. > That can > live-lock, if the GPU produces IRQs faster than the kernel can process > them. Therefore I was looking at rptr and wptr to drain only IRQs that > were already in the queue when the drain call was made. That also > ensures that the wait time is bounded and should be short (unless the > ring size is huge). Correct as well, but the problem here is that Jonathan's implementation is not even remotely correct. See when you look at the rptr and wptr you can't be sure that they haven't wrapped around between two looks. What you could do is look at both the rptr as well as the original wptr, but that is tricky. Regards, Christian. > > Regards, > Felix > > >> Regards, >> Christian. >> >>> Regards, >>> Felix >>> >>> >>>>>> Suggested-by: Felix Kuehling <felix.kuehling@amd.com> >>>>>> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com> >>>>>> --- >>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 46 >>>>> +++++++++++++++++++++++++- >>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 2 ++ >>>>>> 2 files changed, 47 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>>> index dc852af4f3b7..cae50af9559d 100644 >>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>>> @@ -22,7 +22,7 @@ >>>>>> */ >>>>>> >>>>>> #include <linux/dma-mapping.h> >>>>>> - >>>>>> +#include <linux/processor.h> >>>>>> #include "amdgpu.h" >>>>>> #include "amdgpu_ih.h" >>>>>> >>>>>> @@ -160,6 +160,50 @@ void amdgpu_ih_ring_write(struct >>>>> amdgpu_ih_ring *ih, const uint32_t *iv, >>>>>> } >>>>>> } >>>>>> >>>>>> +/** >>>>>> + * amdgpu_ih_wait_on_checkpoint_process - wait to process IVs up to >>>>>> +checkpoint >>>>>> + * >>>>>> + * @adev: amdgpu_device pointer >>>>>> + * @ih: ih ring to process >>>>>> + * >>>>>> + * Used to ensure ring has processed IVs up to the checkpoint write >>>>> pointer. >>>>>> + */ >>>>>> +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device >>>>> *adev, >>>>>> +struct amdgpu_ih_ring *ih) >>>>>> +{ >>>>>> +u32 prev_rptr, cur_rptr, checkpoint_wptr; >>>>>> + >>>>>> +if (!ih->enabled || adev->shutdown) >>>>>> +return -ENODEV; >>>>>> + >>>>>> +cur_rptr = READ_ONCE(ih->rptr); >>>>>> +/* Order read of current rptr with checktpoint wptr. */ >>>>>> +mb(); >>>>>> +checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); >>>>>> + >>>>>> +/* allow rptr to wrap around */ >>>>>> +if (cur_rptr > checkpoint_wptr) { >>>>>> +spin_begin(); >>>>>> +do { >>>>>> +spin_cpu_relax(); >>>>>> +prev_rptr = cur_rptr; >>>>>> +cur_rptr = READ_ONCE(ih->rptr); >>>>>> +} while (cur_rptr >= prev_rptr); >>>>>> +spin_end(); >>>>> That's a certain NAK since it busy waits for IH processing. We need >>>>> some >>>>> event to trigger here. >>>> The function is meant to be just a waiter up to the checkpoint. >>>> There's a need to guarantee that "stale" interrupts have been >>>> processed on check before doing other stuff after call. >>>> The description could be improved to clarify that. >>>> >>>> Would busy waiting only on a locked ring help? I assume an unlocked >>>> ring means nothing to process so no need to wait and we can exit >>>> early. Or is it better to just to process the entries up to the >>>> checkpoint (maybe adjust amdgpu_ih_process for this need like adding >>>> a bool arg to skip restart or something)? >>>> >>>> Thanks, >>>> >>>> Jon >>>> >>>>>> +} >>>>>> + >>>>>> +/* wait for rptr to catch up to or pass checkpoint. */ >>>>>> +spin_begin(); >>>>>> +do { >>>>>> +spin_cpu_relax(); >>>>>> +prev_rptr = cur_rptr; >>>>>> +cur_rptr = READ_ONCE(ih->rptr); >>>>>> +} while (cur_rptr >= prev_rptr && cur_rptr < checkpoint_wptr); >>>>> Same of course here. >>>>> >>>>> Christian. >>>>> >>>>>> +spin_end(); >>>>>> + >>>>>> +return 0; >>>>>> +} >>>>>> + >>>>>> /** >>>>>> * amdgpu_ih_process - interrupt handler >>>>>> * >>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>>> index 6ed4a85fc7c3..6817f0a812d2 100644 >>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>>> @@ -87,6 +87,8 @@ int amdgpu_ih_ring_init(struct amdgpu_device *adev, >>>>> struct amdgpu_ih_ring *ih, >>>>>> void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct >>>>> amdgpu_ih_ring *ih); >>>>>> void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const >>>>>> uint32_t *iv, >>>>>> unsigned int num_dw); >>>>>> +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device >>>>> *adev, >>>>>> +struct amdgpu_ih_ring *ih); >>>>>> int amdgpu_ih_process(struct amdgpu_device *adev, struct >>>>> amdgpu_ih_ring *ih); >>>>>> void amdgpu_ih_decode_iv_helper(struct amdgpu_device *adev, >>>>>> struct amdgpu_ih_ring *ih, >>> _______________________________________________ >>> amd-gfx mailing list >>> amd-gfx@lists.freedesktop.org >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cfelix.kuehling%40amd.com%7C84d85e54bdcb4593e07f08d8d994be77%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637498580167313193%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=RvRHB9l4O%2BpbpogUFKUmnMGkqKnecwQCYRHrkxICDqU%3D&reserved=0 >>> _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: add ih call to process until checkpoint 2021-02-25 16:48 ` Christian König @ 2021-02-25 18:33 ` Felix Kuehling 2021-02-25 20:05 ` Christian König 0 siblings, 1 reply; 10+ messages in thread From: Felix Kuehling @ 2021-02-25 18:33 UTC (permalink / raw) To: Christian König, Christian König, Kim, Jonathan, amd-gfx Cc: Yang, Philip Am 2021-02-25 um 11:48 a.m. schrieb Christian König: > > > Am 25.02.21 um 16:35 schrieb Felix Kuehling: >> Am 2021-02-25 um 8:53 a.m. schrieb Christian König: >>> >>> Am 25.02.21 um 04:15 schrieb Felix Kuehling: >>>> On 2021-02-24 10:54 a.m., Kim, Jonathan wrote: >>>>> [AMD Official Use Only - Internal Distribution Only] >>>>> >>>>>> -----Original Message----- >>>>>> From: Koenig, Christian <Christian.Koenig@amd.com> >>>>>> Sent: Wednesday, February 24, 2021 4:17 AM >>>>>> To: Kim, Jonathan <Jonathan.Kim@amd.com>; amd- >>>>>> gfx@lists.freedesktop.org >>>>>> Cc: Yang, Philip <Philip.Yang@amd.com>; Kuehling, Felix >>>>>> <Felix.Kuehling@amd.com> >>>>>> Subject: Re: [PATCH] drm/amdgpu: add ih call to process until >>>>>> checkpoint >>>>>> >>>>>> Am 23.02.21 um 22:10 schrieb Jonathan Kim: >>>>>>> Add IH function to allow caller to process ring entries until the >>>>>>> checkpoint write pointer. >>>>>> This needs a better description of what this will be used for. >>>>> Felix or Philip could elaborate better for HMM needs. >>>>> Debugging tools requires this but it's in experimental mode at the >>>>> moment so probably not the best place to describe here. >>>> On the HMM side we're planning to use this to drain pending page >>>> fault interrupts before we unmap memory. That should address phantom >>>> VM faults after memory is unmapped. >>> Thought so. I suggest to use a wait_event() here which on the waiter >>> side checks ih->lock and add a wake_up_all() at the end of >>> amdgpu_ih_process. >> Right. I thought about that and it should be easy to add. The reason to >> suggest busy waiting first is, that interrupt processing is supposed to >> be fast. The IRQ handler itself doesn't sleep. So I'd expect the wait >> time to be short enough that sleeping and scheduling is not worth it. > > Well the page fault IRQs are processed in a work item, so we busy wait > for another thread here and not interrupt context. Good point. > > This in turn can lead to starvation of the work handler and so a life > lock as well. > >> >> >>> I won't touch rptr or wptr at all for this. >> Not sure what's your idea here, using ih->lock. Is it to completely >> drain all IRQs until the IH ring is completely empty? > > Correct. > >> That can >> live-lock, if the GPU produces IRQs faster than the kernel can process >> them. Therefore I was looking at rptr and wptr to drain only IRQs that >> were already in the queue when the drain call was made. That also >> ensures that the wait time is bounded and should be short (unless the >> ring size is huge). > > Correct as well, but the problem here is that Jonathan's > implementation is not even remotely correct. > > See when you look at the rptr and wptr you can't be sure that they > haven't wrapped around between two looks. > > What you could do is look at both the rptr as well as the original > wptr, but that is tricky. The code in Jon's patch was suggested by me. I check for wrapping by comparing rptr with the rptr from the previous loop iteration. Comparing rptr with wptr you can never be sure whether rptr has already passed wptr, or whether rptr has to wrap first. I could see a compromise where we sleep and wake up the waiting threads when 1. the IH ring is empty 2. the IH rptr wraps That should be good enough to ensure a quick response in the common case (no interrupt storm), and a reasonable response in the interrupt storm case. But then it's still not clear what's the correct condition for checking that the interrupts the caller cares about have been drained. Looking at just rptr and wptr is always ambiguous. Maybe we could use timestamps of the last processed interrupt? Those 48-bit time stamps wrap much less frequently. The idea is this: * At the start get the timestamp of the last written IH ring entry (checkpoint) * Wait until the last_processed IH timestamp passes the checkpoint: sign_extend(last_processed - checkpoint) >= 0 Regards, Felix > > Regards, > Christian. > >> >> Regards, >> Felix >> >> >>> Regards, >>> Christian. >>> >>>> Regards, >>>> Felix >>>> >>>> >>>>>>> Suggested-by: Felix Kuehling <felix.kuehling@amd.com> >>>>>>> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com> >>>>>>> --- >>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 46 >>>>>> +++++++++++++++++++++++++- >>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 2 ++ >>>>>>> 2 files changed, 47 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>>>> index dc852af4f3b7..cae50af9559d 100644 >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>>>> @@ -22,7 +22,7 @@ >>>>>>> */ >>>>>>> >>>>>>> #include <linux/dma-mapping.h> >>>>>>> - >>>>>>> +#include <linux/processor.h> >>>>>>> #include "amdgpu.h" >>>>>>> #include "amdgpu_ih.h" >>>>>>> >>>>>>> @@ -160,6 +160,50 @@ void amdgpu_ih_ring_write(struct >>>>>> amdgpu_ih_ring *ih, const uint32_t *iv, >>>>>>> } >>>>>>> } >>>>>>> >>>>>>> +/** >>>>>>> + * amdgpu_ih_wait_on_checkpoint_process - wait to process IVs >>>>>>> up to >>>>>>> +checkpoint >>>>>>> + * >>>>>>> + * @adev: amdgpu_device pointer >>>>>>> + * @ih: ih ring to process >>>>>>> + * >>>>>>> + * Used to ensure ring has processed IVs up to the checkpoint >>>>>>> write >>>>>> pointer. >>>>>>> + */ >>>>>>> +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device >>>>>> *adev, >>>>>>> +struct amdgpu_ih_ring *ih) >>>>>>> +{ >>>>>>> +u32 prev_rptr, cur_rptr, checkpoint_wptr; >>>>>>> + >>>>>>> +if (!ih->enabled || adev->shutdown) >>>>>>> +return -ENODEV; >>>>>>> + >>>>>>> +cur_rptr = READ_ONCE(ih->rptr); >>>>>>> +/* Order read of current rptr with checktpoint wptr. */ >>>>>>> +mb(); >>>>>>> +checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); >>>>>>> + >>>>>>> +/* allow rptr to wrap around */ >>>>>>> +if (cur_rptr > checkpoint_wptr) { >>>>>>> +spin_begin(); >>>>>>> +do { >>>>>>> +spin_cpu_relax(); >>>>>>> +prev_rptr = cur_rptr; >>>>>>> +cur_rptr = READ_ONCE(ih->rptr); >>>>>>> +} while (cur_rptr >= prev_rptr); >>>>>>> +spin_end(); >>>>>> That's a certain NAK since it busy waits for IH processing. We need >>>>>> some >>>>>> event to trigger here. >>>>> The function is meant to be just a waiter up to the checkpoint. >>>>> There's a need to guarantee that "stale" interrupts have been >>>>> processed on check before doing other stuff after call. >>>>> The description could be improved to clarify that. >>>>> >>>>> Would busy waiting only on a locked ring help? I assume an unlocked >>>>> ring means nothing to process so no need to wait and we can exit >>>>> early. Or is it better to just to process the entries up to the >>>>> checkpoint (maybe adjust amdgpu_ih_process for this need like adding >>>>> a bool arg to skip restart or something)? >>>>> >>>>> Thanks, >>>>> >>>>> Jon >>>>> >>>>>>> +} >>>>>>> + >>>>>>> +/* wait for rptr to catch up to or pass checkpoint. */ >>>>>>> +spin_begin(); >>>>>>> +do { >>>>>>> +spin_cpu_relax(); >>>>>>> +prev_rptr = cur_rptr; >>>>>>> +cur_rptr = READ_ONCE(ih->rptr); >>>>>>> +} while (cur_rptr >= prev_rptr && cur_rptr < checkpoint_wptr); >>>>>> Same of course here. >>>>>> >>>>>> Christian. >>>>>> >>>>>>> +spin_end(); >>>>>>> + >>>>>>> +return 0; >>>>>>> +} >>>>>>> + >>>>>>> /** >>>>>>> * amdgpu_ih_process - interrupt handler >>>>>>> * >>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>>>> index 6ed4a85fc7c3..6817f0a812d2 100644 >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>>>> @@ -87,6 +87,8 @@ int amdgpu_ih_ring_init(struct amdgpu_device >>>>>>> *adev, >>>>>> struct amdgpu_ih_ring *ih, >>>>>>> void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct >>>>>> amdgpu_ih_ring *ih); >>>>>>> void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const >>>>>>> uint32_t *iv, >>>>>>> unsigned int num_dw); >>>>>>> +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device >>>>>> *adev, >>>>>>> +struct amdgpu_ih_ring *ih); >>>>>>> int amdgpu_ih_process(struct amdgpu_device *adev, struct >>>>>> amdgpu_ih_ring *ih); >>>>>>> void amdgpu_ih_decode_iv_helper(struct amdgpu_device *adev, >>>>>>> struct amdgpu_ih_ring *ih, >>>> _______________________________________________ >>>> amd-gfx mailing list >>>> amd-gfx@lists.freedesktop.org >>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cfelix.kuehling%40amd.com%7C84d85e54bdcb4593e07f08d8d994be77%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637498580167313193%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=RvRHB9l4O%2BpbpogUFKUmnMGkqKnecwQCYRHrkxICDqU%3D&reserved=0 >>>> >>>> > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: add ih call to process until checkpoint 2021-02-25 18:33 ` Felix Kuehling @ 2021-02-25 20:05 ` Christian König 0 siblings, 0 replies; 10+ messages in thread From: Christian König @ 2021-02-25 20:05 UTC (permalink / raw) To: Felix Kuehling, Christian König, Kim, Jonathan, amd-gfx; +Cc: Yang, Philip Am 25.02.21 um 19:33 schrieb Felix Kuehling: > [SNIP] >> This in turn can lead to starvation of the work handler and so a life >> lock as well. >> >>> >>>> I won't touch rptr or wptr at all for this. >>> Not sure what's your idea here, using ih->lock. Is it to completely >>> drain all IRQs until the IH ring is completely empty? >> Correct. >> >>> That can >>> live-lock, if the GPU produces IRQs faster than the kernel can process >>> them. Therefore I was looking at rptr and wptr to drain only IRQs that >>> were already in the queue when the drain call was made. That also >>> ensures that the wait time is bounded and should be short (unless the >>> ring size is huge). >> Correct as well, but the problem here is that Jonathan's >> implementation is not even remotely correct. >> >> See when you look at the rptr and wptr you can't be sure that they >> haven't wrapped around between two looks. >> >> What you could do is look at both the rptr as well as the original >> wptr, but that is tricky. > The code in Jon's patch was suggested by me. I check for wrapping by > comparing rptr with the rptr from the previous loop iteration. Comparing > rptr with wptr you can never be sure whether rptr has already passed > wptr, or whether rptr has to wrap first. Exactly that's my concern as well. > > I could see a compromise where we sleep and wake up the waiting threads when > > 1. the IH ring is empty > 2. the IH rptr wraps > > That should be good enough to ensure a quick response in the common case > (no interrupt storm), and a reasonable response in the interrupt storm case. > > But then it's still not clear what's the correct condition for checking > that the interrupts the caller cares about have been drained. Looking at > just rptr and wptr is always ambiguous. Maybe we could use timestamps of > the last processed interrupt? Those 48-bit time stamps wrap much less > frequently. The idea is this: > > * At the start get the timestamp of the last written IH ring entry > (checkpoint) > * Wait until the last_processed IH timestamp passes the checkpoint: > sign_extend(last_processed - checkpoint) >= 0 Yeah that could work. Alternatively we could just have a rptr wrap around counter. This way you do something like this: 1. get the wrap around counter. 2. get wptr 3. get rptr 4. compare the wrap around counter with the old one, if it has changed start over with #1 5. Use wrap around counter and rptr/wptr to come up with 64bit values. 6. Compare wptr with rptr/wrap around counter until we are sure the IHs are processed. Regards, Christian. > > Regards, > Felix > > >> Regards, >> Christian. >> >>> Regards, >>> Felix >>> >>> >>>> Regards, >>>> Christian. >>>> >>>>> Regards, >>>>> Felix >>>>> >>>>> >>>>>>>> Suggested-by: Felix Kuehling <felix.kuehling@amd.com> >>>>>>>> Signed-off-by: Jonathan Kim <jonathan.kim@amd.com> >>>>>>>> --- >>>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c | 46 >>>>>>> +++++++++++++++++++++++++- >>>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h | 2 ++ >>>>>>>> 2 files changed, 47 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>>>>> index dc852af4f3b7..cae50af9559d 100644 >>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c >>>>>>>> @@ -22,7 +22,7 @@ >>>>>>>> */ >>>>>>>> >>>>>>>> #include <linux/dma-mapping.h> >>>>>>>> - >>>>>>>> +#include <linux/processor.h> >>>>>>>> #include "amdgpu.h" >>>>>>>> #include "amdgpu_ih.h" >>>>>>>> >>>>>>>> @@ -160,6 +160,50 @@ void amdgpu_ih_ring_write(struct >>>>>>> amdgpu_ih_ring *ih, const uint32_t *iv, >>>>>>>> } >>>>>>>> } >>>>>>>> >>>>>>>> +/** >>>>>>>> + * amdgpu_ih_wait_on_checkpoint_process - wait to process IVs >>>>>>>> up to >>>>>>>> +checkpoint >>>>>>>> + * >>>>>>>> + * @adev: amdgpu_device pointer >>>>>>>> + * @ih: ih ring to process >>>>>>>> + * >>>>>>>> + * Used to ensure ring has processed IVs up to the checkpoint >>>>>>>> write >>>>>>> pointer. >>>>>>>> + */ >>>>>>>> +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device >>>>>>> *adev, >>>>>>>> +struct amdgpu_ih_ring *ih) >>>>>>>> +{ >>>>>>>> +u32 prev_rptr, cur_rptr, checkpoint_wptr; >>>>>>>> + >>>>>>>> +if (!ih->enabled || adev->shutdown) >>>>>>>> +return -ENODEV; >>>>>>>> + >>>>>>>> +cur_rptr = READ_ONCE(ih->rptr); >>>>>>>> +/* Order read of current rptr with checktpoint wptr. */ >>>>>>>> +mb(); >>>>>>>> +checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih); >>>>>>>> + >>>>>>>> +/* allow rptr to wrap around */ >>>>>>>> +if (cur_rptr > checkpoint_wptr) { >>>>>>>> +spin_begin(); >>>>>>>> +do { >>>>>>>> +spin_cpu_relax(); >>>>>>>> +prev_rptr = cur_rptr; >>>>>>>> +cur_rptr = READ_ONCE(ih->rptr); >>>>>>>> +} while (cur_rptr >= prev_rptr); >>>>>>>> +spin_end(); >>>>>>> That's a certain NAK since it busy waits for IH processing. We need >>>>>>> some >>>>>>> event to trigger here. >>>>>> The function is meant to be just a waiter up to the checkpoint. >>>>>> There's a need to guarantee that "stale" interrupts have been >>>>>> processed on check before doing other stuff after call. >>>>>> The description could be improved to clarify that. >>>>>> >>>>>> Would busy waiting only on a locked ring help? I assume an unlocked >>>>>> ring means nothing to process so no need to wait and we can exit >>>>>> early. Or is it better to just to process the entries up to the >>>>>> checkpoint (maybe adjust amdgpu_ih_process for this need like adding >>>>>> a bool arg to skip restart or something)? >>>>>> >>>>>> Thanks, >>>>>> >>>>>> Jon >>>>>> >>>>>>>> +} >>>>>>>> + >>>>>>>> +/* wait for rptr to catch up to or pass checkpoint. */ >>>>>>>> +spin_begin(); >>>>>>>> +do { >>>>>>>> +spin_cpu_relax(); >>>>>>>> +prev_rptr = cur_rptr; >>>>>>>> +cur_rptr = READ_ONCE(ih->rptr); >>>>>>>> +} while (cur_rptr >= prev_rptr && cur_rptr < checkpoint_wptr); >>>>>>> Same of course here. >>>>>>> >>>>>>> Christian. >>>>>>> >>>>>>>> +spin_end(); >>>>>>>> + >>>>>>>> +return 0; >>>>>>>> +} >>>>>>>> + >>>>>>>> /** >>>>>>>> * amdgpu_ih_process - interrupt handler >>>>>>>> * >>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>>>>> index 6ed4a85fc7c3..6817f0a812d2 100644 >>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h >>>>>>>> @@ -87,6 +87,8 @@ int amdgpu_ih_ring_init(struct amdgpu_device >>>>>>>> *adev, >>>>>>> struct amdgpu_ih_ring *ih, >>>>>>>> void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct >>>>>>> amdgpu_ih_ring *ih); >>>>>>>> void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const >>>>>>>> uint32_t *iv, >>>>>>>> unsigned int num_dw); >>>>>>>> +int amdgpu_ih_wait_on_checkpoint_process(struct amdgpu_device >>>>>>> *adev, >>>>>>>> +struct amdgpu_ih_ring *ih); >>>>>>>> int amdgpu_ih_process(struct amdgpu_device *adev, struct >>>>>>> amdgpu_ih_ring *ih); >>>>>>>> void amdgpu_ih_decode_iv_helper(struct amdgpu_device *adev, >>>>>>>> struct amdgpu_ih_ring *ih, >>>>> _______________________________________________ >>>>> amd-gfx mailing list >>>>> amd-gfx@lists.freedesktop.org >>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cfelix.kuehling%40amd.com%7C84d85e54bdcb4593e07f08d8d994be77%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637498580167313193%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=RvRHB9l4O%2BpbpogUFKUmnMGkqKnecwQCYRHrkxICDqU%3D&reserved=0 >>>>> >>>>> _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-02-25 20:05 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-23 21:10 [PATCH] drm/amdgpu: add ih call to process until checkpoint Jonathan Kim 2021-02-23 22:45 ` Andrey Grodzovsky 2021-02-24 9:16 ` Christian König 2021-02-24 15:54 ` Kim, Jonathan 2021-02-25 3:15 ` Felix Kuehling 2021-02-25 13:53 ` Christian König 2021-02-25 15:35 ` Felix Kuehling 2021-02-25 16:48 ` Christian König 2021-02-25 18:33 ` Felix Kuehling 2021-02-25 20:05 ` Christian König
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.