<!DOCTYPE html> <html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body> Am 08.10.24 um 20:43 schrieb Khatri, Sunil: <blockquote type="cite" cite="mid:ad51bbc1-4e5d-e5c0-60b7-35cedbd77d9a@amd.com"> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> <div class="moz-cite-prefix">On 10/8/2024 11:41 PM, Christian König wrote: </div> <blockquote type="cite" cite="mid:20241008181134.1350-2-christian.koenig@amd.com"> <pre class="moz-quote-pre" wrap="">Stop masking the wptr and decrementing the count_dw while writing into the ring buffer. We can do that all at once while pushing the changes to the HW. Signed-off-by: Christian König <a class="moz-txt-link-rfc2396E" href="mailto:christian.koenig@amd.com" moz-do-not-send="true"><christian.koenig@amd.com></a> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 11 +++++------ drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 4 ---- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c index a6e28fe3f8d6..364466d6d3de 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c @@ -90,12 +90,11 @@ int amdgpu_ring_alloc(struct amdgpu_ring *ring, unsigned int ndw) if (WARN_ON_ONCE(ndw > ring->max_dw)) return -ENOMEM; - ring->count_dw = ndw; - ring->wptr_old = ring->wptr; - if (ring->funcs->begin_use) ring->funcs->begin_use(ring); + ring->count_dw = ndw; + ring->wptr_old = ring->wptr;</pre> </blockquote> Does moving these after begin_use makes any difference? Otherwise looks good to me. </blockquote> Yes, it does. The VCN begin_use() callback modifies the wptr on resume. So we can only safe wptr_old after making the callback. <blockquote type="cite" cite="mid:ad51bbc1-4e5d-e5c0-60b7-35cedbd77d9a@amd.com">  <span style="font-size:12.0pt;font-family:"Aptos",sans-serif; mso-ascii-theme-font:minor-latin;mso-fareast-font-family:Aptos;mso-fareast-theme-font: minor-latin;mso-hansi-theme-font:minor-latin;mso-bidi-font-family:Aptos; color:black;mso-ansi-language:EN-US;mso-fareast-language:EN-US;mso-bidi-language: AR-SA">Reviewed-by: Sunil Khatri <a href="mailto:sunil.khatri@amd.com" moz-do-not-send="true"><sunil.khatri@amd.com></a> </blockquote> Thanks, Christian. <blockquote type="cite" cite="mid:ad51bbc1-4e5d-e5c0-60b7-35cedbd77d9a@amd.com"> <span style="font-size:12.0pt;font-family:"Aptos",sans-serif; mso-ascii-theme-font:minor-latin;mso-fareast-font-family:Aptos;mso-fareast-theme-font: minor-latin;mso-hansi-theme-font:minor-latin;mso-bidi-font-family:Aptos; color:black;mso-ansi-language:EN-US;mso-fareast-language:EN-US;mso-bidi-language: AR-SA"> Regards Sunil Khatri <span style="font-size:12.0pt;font-family:"Aptos",sans-serif; mso-ascii-theme-font:minor-latin;mso-fareast-font-family:Aptos;mso-fareast-theme-font: minor-latin;mso-hansi-theme-font:minor-latin;mso-bidi-font-family:Aptos; color:black;mso-ansi-language:EN-US;mso-fareast-language:EN-US;mso-bidi-language: AR-SA"> <blockquote type="cite" cite="mid:20241008181134.1350-2-christian.koenig@amd.com"> <pre class="moz-quote-pre" wrap=""> return 0; } @@ -122,8 +121,6 @@ void amdgpu_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count) memset32(ring->ring, ring->funcs->nop, chunk2); ring->wptr += count; - ring->wptr &= ring->ptr_mask; - ring->count_dw -= count; } /** @@ -153,9 +150,11 @@ void amdgpu_ring_commit(struct amdgpu_ring *ring) { uint32_t count; - if (ring->count_dw < 0) + if ((ring->wptr - ring->wptr_old) > ring->count_dw) DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n"); + ring->wptr &= ring->ptr_mask; + /* We pad to match fetch size */ count = ring->funcs->align_mask + 1 - (ring->wptr & ring->funcs->align_mask); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h index 36fc9578c53c..439793206b89 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h @@ -378,8 +378,6 @@ static inline void amdgpu_ring_clear_ring(struct amdgpu_ring *ring) static inline void amdgpu_ring_write(struct amdgpu_ring *ring, uint32_t v) { ring->ring[ring->wptr++ & ring->buf_mask] = v; - ring->wptr &= ring->ptr_mask; - ring->count_dw--; } static inline void amdgpu_ring_write_multiple(struct amdgpu_ring *ring, @@ -403,8 +401,6 @@ static inline void amdgpu_ring_write_multiple(struct amdgpu_ring *ring, } ring->wptr += count_dw; - ring->wptr &= ring->ptr_mask; - ring->count_dw -= count_dw; } /** </pre> </blockquote> </blockquote> </body> </html>