Re: [PATCH] drm/amdgpu: do not modify ring before doing ring validation
On 2/25/20 12:08 PM, Christian König wrote: Am 25.02.20 um 12:03 schrieb Nirmoy: On 2/25/20 11:52 AM, Christian König wrote: Am 25.02.20 um 11:39 schrieb Nirmoy Das: changing ring->sched.ready should be done only if the ring is initialized I don't think that this makes much difference. When the rings are freed the hardware and software stack needs to be disabled quite some time before. Yes it doesn't make any difference. I wanted amdgpu_ring_fini() to look bit cleaner. Ok, you should note that in the subject and commit message. Something like "cleanup amdgpu_ring_fini" / "Check the prerequisites before the actual operation". Otherwise if that is not clearly state somebody might think that this is a bug fix and pick it up for backporting and we probably don't want that. Sorry, true the subject line is confusing with the bug :). I will modify and resend. Regards, Nirmoy Regards, Christian. Regards, Nirmoy Christian. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c index 539be138260e..18e11b0fdc3e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c @@ -344,12 +344,13 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring, */ void amdgpu_ring_fini(struct amdgpu_ring *ring) { - ring->sched.ready = false; /* Not to finish a ring which is not initialized */ if (!(ring->adev) || !(ring->adev->rings[ring->idx])) return; + ring->sched.ready = false; + amdgpu_device_wb_free(ring->adev, ring->rptr_offs); amdgpu_device_wb_free(ring->adev, ring->wptr_offs); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: do not modify ring before doing ring validation
Am 25.02.20 um 12:03 schrieb Nirmoy: On 2/25/20 11:52 AM, Christian König wrote: Am 25.02.20 um 11:39 schrieb Nirmoy Das: changing ring->sched.ready should be done only if the ring is initialized I don't think that this makes much difference. When the rings are freed the hardware and software stack needs to be disabled quite some time before. Yes it doesn't make any difference. I wanted amdgpu_ring_fini() to look bit cleaner. Ok, you should note that in the subject and commit message. Something like "cleanup amdgpu_ring_fini" / "Check the prerequisites before the actual operation". Otherwise if that is not clearly state somebody might think that this is a bug fix and pick it up for backporting and we probably don't want that. Regards, Christian. Regards, Nirmoy Christian. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c index 539be138260e..18e11b0fdc3e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c @@ -344,12 +344,13 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring, */ void amdgpu_ring_fini(struct amdgpu_ring *ring) { - ring->sched.ready = false; /* Not to finish a ring which is not initialized */ if (!(ring->adev) || !(ring->adev->rings[ring->idx])) return; + ring->sched.ready = false; + amdgpu_device_wb_free(ring->adev, ring->rptr_offs); amdgpu_device_wb_free(ring->adev, ring->wptr_offs); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: do not modify ring before doing ring validation
On 2/25/20 11:52 AM, Christian König wrote: Am 25.02.20 um 11:39 schrieb Nirmoy Das: changing ring->sched.ready should be done only if the ring is initialized I don't think that this makes much difference. When the rings are freed the hardware and software stack needs to be disabled quite some time before. Yes it doesn't make any difference. I wanted amdgpu_ring_fini() to look bit cleaner. Regards, Nirmoy Christian. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c index 539be138260e..18e11b0fdc3e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c @@ -344,12 +344,13 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring, */ void amdgpu_ring_fini(struct amdgpu_ring *ring) { - ring->sched.ready = false; /* Not to finish a ring which is not initialized */ if (!(ring->adev) || !(ring->adev->rings[ring->idx])) return; + ring->sched.ready = false; + amdgpu_device_wb_free(ring->adev, ring->rptr_offs); amdgpu_device_wb_free(ring->adev, ring->wptr_offs); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: do not modify ring before doing ring validation
Am 25.02.20 um 11:39 schrieb Nirmoy Das: changing ring->sched.ready should be done only if the ring is initialized I don't think that this makes much difference. When the rings are freed the hardware and software stack needs to be disabled quite some time before. Christian. Signed-off-by: Nirmoy Das --- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c index 539be138260e..18e11b0fdc3e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c @@ -344,12 +344,13 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring, */ void amdgpu_ring_fini(struct amdgpu_ring *ring) { - ring->sched.ready = false; /* Not to finish a ring which is not initialized */ if (!(ring->adev) || !(ring->adev->rings[ring->idx])) return; + ring->sched.ready = false; + amdgpu_device_wb_free(ring->adev, ring->rptr_offs); amdgpu_device_wb_free(ring->adev, ring->wptr_offs); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: do not modify ring before doing ring validation
changing ring->sched.ready should be done only if the ring is initialized Signed-off-by: Nirmoy Das --- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c index 539be138260e..18e11b0fdc3e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c @@ -344,12 +344,13 @@ int amdgpu_ring_init(struct amdgpu_device *adev, struct amdgpu_ring *ring, */ void amdgpu_ring_fini(struct amdgpu_ring *ring) { - ring->sched.ready = false; /* Not to finish a ring which is not initialized */ if (!(ring->adev) || !(ring->adev->rings[ring->idx])) return; + ring->sched.ready = false; + amdgpu_device_wb_free(ring->adev, ring->rptr_offs); amdgpu_device_wb_free(ring->adev, ring->wptr_offs); -- 2.25.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx