* [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init
@ 2019-10-01 17:16 Gustavo A. R. Silva
2019-10-01 21:29 ` Liu, Leo
0 siblings, 1 reply; 10+ messages in thread
From: Gustavo A. R. Silva @ 2019-10-01 17:16 UTC (permalink / raw)
To: Alex Deucher, Christian König, David (ChunMing) Zhou,
David Airlie, Daniel Vetter, Leo Liu
Cc: amd-gfx, dri-devel, linux-kernel, Gustavo A. R. Silva
Notice that there is a *continue* statement in the middle of the
for loop and that prevents the code below from ever being reached:
r = amdgpu_ring_test_ring(ring);
if (r) {
ring->sched.ready = false;
goto done;
}
Fix this by removing the continue statement and updating ring->sched.ready
to true before calling amdgpu_ring_test_ring(ring).
Notice that this fix is based on
commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)")
Addresses-Coverity-ID 1485608 ("Structurally dead code")
Fixes: 28c17d72072b ("drm/amdgpu: add VCN2.5 basic supports")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
Any feedback is greatly appreciated.
drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
index 395c2259f979..47b0dcd59e13 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
@@ -258,6 +258,7 @@ static int vcn_v2_5_hw_init(void *handle)
adev->nbio_funcs->vcn_doorbell_range(adev, ring->use_doorbell,
ring->doorbell_index, j);
+ ring->sched.ready = true;
r = amdgpu_ring_test_ring(ring);
if (r) {
ring->sched.ready = false;
@@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle)
for (i = 0; i < adev->vcn.num_enc_rings; ++i) {
ring = &adev->vcn.inst[j].ring_enc[i];
- ring->sched.ready = false;
- continue;
+ ring->sched.ready = true;
r = amdgpu_ring_test_ring(ring);
if (r) {
ring->sched.ready = false;
@@ -276,6 +276,7 @@ static int vcn_v2_5_hw_init(void *handle)
}
ring = &adev->vcn.inst[j].ring_jpeg;
+ ring->sched.ready = true;
r = amdgpu_ring_test_ring(ring);
if (r) {
ring->sched.ready = false;
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init 2019-10-01 17:16 [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init Gustavo A. R. Silva @ 2019-10-01 21:29 ` Liu, Leo 0 siblings, 0 replies; 10+ messages in thread From: Liu, Leo @ 2019-10-01 21:29 UTC (permalink / raw) To: Gustavo A. R. Silva, Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing), David Airlie, Daniel Vetter Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On 2019-10-01 1:16 p.m., Gustavo A. R. Silva wrote: > Notice that there is a *continue* statement in the middle of the > for loop and that prevents the code below from ever being reached: > > r = amdgpu_ring_test_ring(ring); > if (r) { > ring->sched.ready = false; > goto done; > } > > Fix this by removing the continue statement and updating ring->sched.ready > to true before calling amdgpu_ring_test_ring(ring). > > Notice that this fix is based on > commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)") > > Addresses-Coverity-ID 1485608 ("Structurally dead code") > Fixes: 28c17d72072b ("drm/amdgpu: add VCN2.5 basic supports") > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > --- > > Any feedback is greatly appreciated. > > drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c > index 395c2259f979..47b0dcd59e13 100644 > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c > @@ -258,6 +258,7 @@ static int vcn_v2_5_hw_init(void *handle) > adev->nbio_funcs->vcn_doorbell_range(adev, ring->use_doorbell, > ring->doorbell_index, j); > > + ring->sched.ready = true; This is redundant. all the sched->ready is initialized as true, please refer to drm_sched_init() > r = amdgpu_ring_test_ring(ring); > if (r) { > ring->sched.ready = false; > @@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle) > > for (i = 0; i < adev->vcn.num_enc_rings; ++i) { > ring = &adev->vcn.inst[j].ring_enc[i]; > - ring->sched.ready = false; > - continue; > + ring->sched.ready = true; Because the VCN 2.5 FW still has issue for encode, so we have it disabled here. Regards, Leo > r = amdgpu_ring_test_ring(ring); > if (r) { > ring->sched.ready = false; > @@ -276,6 +276,7 @@ static int vcn_v2_5_hw_init(void *handle) > } > > ring = &adev->vcn.inst[j].ring_jpeg; > + ring->sched.ready = true; > r = amdgpu_ring_test_ring(ring); > if (r) { > ring->sched.ready = false; _______________________________________________ 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: fix structurally dead code vcn_v2_5_hw_init @ 2019-10-01 21:29 ` Liu, Leo 0 siblings, 0 replies; 10+ messages in thread From: Liu, Leo @ 2019-10-01 21:29 UTC (permalink / raw) To: Gustavo A. R. Silva, Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing), David Airlie, Daniel Vetter Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org On 2019-10-01 1:16 p.m., Gustavo A. R. Silva wrote: > Notice that there is a *continue* statement in the middle of the > for loop and that prevents the code below from ever being reached: > > r = amdgpu_ring_test_ring(ring); > if (r) { > ring->sched.ready = false; > goto done; > } > > Fix this by removing the continue statement and updating ring->sched.ready > to true before calling amdgpu_ring_test_ring(ring). > > Notice that this fix is based on > commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)") > > Addresses-Coverity-ID 1485608 ("Structurally dead code") > Fixes: 28c17d72072b ("drm/amdgpu: add VCN2.5 basic supports") > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > --- > > Any feedback is greatly appreciated. > > drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c > index 395c2259f979..47b0dcd59e13 100644 > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c > @@ -258,6 +258,7 @@ static int vcn_v2_5_hw_init(void *handle) > adev->nbio_funcs->vcn_doorbell_range(adev, ring->use_doorbell, > ring->doorbell_index, j); > > + ring->sched.ready = true; This is redundant. all the sched->ready is initialized as true, please refer to drm_sched_init() > r = amdgpu_ring_test_ring(ring); > if (r) { > ring->sched.ready = false; > @@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle) > > for (i = 0; i < adev->vcn.num_enc_rings; ++i) { > ring = &adev->vcn.inst[j].ring_enc[i]; > - ring->sched.ready = false; > - continue; > + ring->sched.ready = true; Because the VCN 2.5 FW still has issue for encode, so we have it disabled here. Regards, Leo > r = amdgpu_ring_test_ring(ring); > if (r) { > ring->sched.ready = false; > @@ -276,6 +276,7 @@ static int vcn_v2_5_hw_init(void *handle) > } > > ring = &adev->vcn.inst[j].ring_jpeg; > + ring->sched.ready = true; > r = amdgpu_ring_test_ring(ring); > if (r) { > ring->sched.ready = false; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init 2019-10-01 21:29 ` Liu, Leo (?) @ 2019-10-01 21:43 ` Gustavo A. R. Silva [not found] ` <823b10a3-fe0e-2e8c-02c3-534944dbe6d2-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org> -1 siblings, 1 reply; 10+ messages in thread From: Gustavo A. R. Silva @ 2019-10-01 21:43 UTC (permalink / raw) To: Liu, Leo, Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing), David Airlie, Daniel Vetter Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Hi, On 10/1/19 16:29, Liu, Leo wrote: > > On 2019-10-01 1:16 p.m., Gustavo A. R. Silva wrote: >> Notice that there is a *continue* statement in the middle of the >> for loop and that prevents the code below from ever being reached: >> >> r = amdgpu_ring_test_ring(ring); >> if (r) { >> ring->sched.ready = false; >> goto done; >> } >> >> Fix this by removing the continue statement and updating ring->sched.ready >> to true before calling amdgpu_ring_test_ring(ring). >> >> Notice that this fix is based on >> commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)") >> >> Addresses-Coverity-ID 1485608 ("Structurally dead code") >> Fixes: 28c17d72072b ("drm/amdgpu: add VCN2.5 basic supports") >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >> --- >> >> Any feedback is greatly appreciated. >> >> drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c >> index 395c2259f979..47b0dcd59e13 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c >> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c >> @@ -258,6 +258,7 @@ static int vcn_v2_5_hw_init(void *handle) >> adev->nbio_funcs->vcn_doorbell_range(adev, ring->use_doorbell, >> ring->doorbell_index, j); >> >> + ring->sched.ready = true; > > This is redundant. all the sched->ready is initialized as true, please > refer to drm_sched_init() > I see... so in the following commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)") that line is also redundant? > >> r = amdgpu_ring_test_ring(ring); >> if (r) { >> ring->sched.ready = false; >> @@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle) >> >> for (i = 0; i < adev->vcn.num_enc_rings; ++i) { >> ring = &adev->vcn.inst[j].ring_enc[i]; >> - ring->sched.ready = false; >> - continue; >> + ring->sched.ready = true; > > Because the VCN 2.5 FW still has issue for encode, so we have it > disabled here. > OK. So, maybe we can add a comment pointing that out? Thanks -- Gustavo > >> r = amdgpu_ring_test_ring(ring); >> if (r) { >> ring->sched.ready = false; >> @@ -276,6 +276,7 @@ static int vcn_v2_5_hw_init(void *handle) >> } >> >> ring = &adev->vcn.inst[j].ring_jpeg; >> + ring->sched.ready = true; >> r = amdgpu_ring_test_ring(ring); >> if (r) { >> ring->sched.ready = false; ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <823b10a3-fe0e-2e8c-02c3-534944dbe6d2-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init 2019-10-01 21:43 ` Gustavo A. R. Silva @ 2019-10-01 21:46 ` Liu, Leo 0 siblings, 0 replies; 10+ messages in thread From: Liu, Leo @ 2019-10-01 21:46 UTC (permalink / raw) To: Gustavo A. R. Silva, Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing), David Airlie, Daniel Vetter Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On 2019-10-01 5:43 p.m., Gustavo A. R. Silva wrote: > Hi, > > On 10/1/19 16:29, Liu, Leo wrote: >> On 2019-10-01 1:16 p.m., Gustavo A. R. Silva wrote: >>> Notice that there is a *continue* statement in the middle of the >>> for loop and that prevents the code below from ever being reached: >>> >>> r = amdgpu_ring_test_ring(ring); >>> if (r) { >>> ring->sched.ready = false; >>> goto done; >>> } >>> >>> Fix this by removing the continue statement and updating ring->sched.ready >>> to true before calling amdgpu_ring_test_ring(ring). >>> >>> Notice that this fix is based on >>> commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)") >>> >>> Addresses-Coverity-ID 1485608 ("Structurally dead code") >>> Fixes: 28c17d72072b ("drm/amdgpu: add VCN2.5 basic supports") >>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >>> --- >>> >>> Any feedback is greatly appreciated. >>> >>> drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c >>> index 395c2259f979..47b0dcd59e13 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c >>> @@ -258,6 +258,7 @@ static int vcn_v2_5_hw_init(void *handle) >>> adev->nbio_funcs->vcn_doorbell_range(adev, ring->use_doorbell, >>> ring->doorbell_index, j); >>> >>> + ring->sched.ready = true; >> This is redundant. all the sched->ready is initialized as true, please >> refer to drm_sched_init() >> > I see... so in the following commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)") > that line is also redundant? Yes. >>> r = amdgpu_ring_test_ring(ring); >>> if (r) { >>> ring->sched.ready = false; >>> @@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle) >>> >>> for (i = 0; i < adev->vcn.num_enc_rings; ++i) { >>> ring = &adev->vcn.inst[j].ring_enc[i]; >>> - ring->sched.ready = false; >>> - continue; >>> + ring->sched.ready = true; >> Because the VCN 2.5 FW still has issue for encode, so we have it >> disabled here. >> > OK. So, maybe we can add a comment pointing that out? That could be better. Thanks, Leo > > Thanks > -- > Gustavo > >>> r = amdgpu_ring_test_ring(ring); >>> if (r) { >>> ring->sched.ready = false; >>> @@ -276,6 +276,7 @@ static int vcn_v2_5_hw_init(void *handle) >>> } >>> >>> ring = &adev->vcn.inst[j].ring_jpeg; >>> + ring->sched.ready = true; >>> r = amdgpu_ring_test_ring(ring); >>> if (r) { >>> ring->sched.ready = false; _______________________________________________ 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: fix structurally dead code vcn_v2_5_hw_init @ 2019-10-01 21:46 ` Liu, Leo 0 siblings, 0 replies; 10+ messages in thread From: Liu, Leo @ 2019-10-01 21:46 UTC (permalink / raw) To: Gustavo A. R. Silva, Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing), David Airlie, Daniel Vetter Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org On 2019-10-01 5:43 p.m., Gustavo A. R. Silva wrote: > Hi, > > On 10/1/19 16:29, Liu, Leo wrote: >> On 2019-10-01 1:16 p.m., Gustavo A. R. Silva wrote: >>> Notice that there is a *continue* statement in the middle of the >>> for loop and that prevents the code below from ever being reached: >>> >>> r = amdgpu_ring_test_ring(ring); >>> if (r) { >>> ring->sched.ready = false; >>> goto done; >>> } >>> >>> Fix this by removing the continue statement and updating ring->sched.ready >>> to true before calling amdgpu_ring_test_ring(ring). >>> >>> Notice that this fix is based on >>> commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)") >>> >>> Addresses-Coverity-ID 1485608 ("Structurally dead code") >>> Fixes: 28c17d72072b ("drm/amdgpu: add VCN2.5 basic supports") >>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >>> --- >>> >>> Any feedback is greatly appreciated. >>> >>> drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c >>> index 395c2259f979..47b0dcd59e13 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c >>> @@ -258,6 +258,7 @@ static int vcn_v2_5_hw_init(void *handle) >>> adev->nbio_funcs->vcn_doorbell_range(adev, ring->use_doorbell, >>> ring->doorbell_index, j); >>> >>> + ring->sched.ready = true; >> This is redundant. all the sched->ready is initialized as true, please >> refer to drm_sched_init() >> > I see... so in the following commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)") > that line is also redundant? Yes. >>> r = amdgpu_ring_test_ring(ring); >>> if (r) { >>> ring->sched.ready = false; >>> @@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle) >>> >>> for (i = 0; i < adev->vcn.num_enc_rings; ++i) { >>> ring = &adev->vcn.inst[j].ring_enc[i]; >>> - ring->sched.ready = false; >>> - continue; >>> + ring->sched.ready = true; >> Because the VCN 2.5 FW still has issue for encode, so we have it >> disabled here. >> > OK. So, maybe we can add a comment pointing that out? That could be better. Thanks, Leo > > Thanks > -- > Gustavo > >>> r = amdgpu_ring_test_ring(ring); >>> if (r) { >>> ring->sched.ready = false; >>> @@ -276,6 +276,7 @@ static int vcn_v2_5_hw_init(void *handle) >>> } >>> >>> ring = &adev->vcn.inst[j].ring_jpeg; >>> + ring->sched.ready = true; >>> r = amdgpu_ring_test_ring(ring); >>> if (r) { >>> ring->sched.ready = false; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init 2019-10-01 21:46 ` Liu, Leo @ 2019-10-01 21:57 ` Gustavo A. R. Silva -1 siblings, 0 replies; 10+ messages in thread From: Gustavo A. R. Silva @ 2019-10-01 21:57 UTC (permalink / raw) To: Liu, Leo, Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing), David Airlie, Daniel Vetter Cc: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org On 10/1/19 16:46, Liu, Leo wrote: >>>> + ring->sched.ready = true; >>> This is redundant. all the sched->ready is initialized as true, please >>> refer to drm_sched_init() >>> >> I see... so in the following commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)") >> that line is also redundant? > > Yes. > OK. > >>>> r = amdgpu_ring_test_ring(ring); >>>> if (r) { >>>> ring->sched.ready = false; >>>> @@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle) >>>> >>>> for (i = 0; i < adev->vcn.num_enc_rings; ++i) { >>>> ring = &adev->vcn.inst[j].ring_enc[i]; >>>> - ring->sched.ready = false; >>>> - continue; >>>> + ring->sched.ready = true; >>> Because the VCN 2.5 FW still has issue for encode, so we have it >>> disabled here. >>> >> OK. So, maybe we can add a comment pointing that out? > > That could be better. > Great. I'm glad it's not a bug. I'll write a patch for that so other people don't waste time taking a look. I appreciate your feedback. Thanks -- Gustavo _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init @ 2019-10-01 21:57 ` Gustavo A. R. Silva 0 siblings, 0 replies; 10+ messages in thread From: Gustavo A. R. Silva @ 2019-10-01 21:57 UTC (permalink / raw) To: Liu, Leo, Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing), David Airlie, Daniel Vetter Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org On 10/1/19 16:46, Liu, Leo wrote: >>>> + ring->sched.ready = true; >>> This is redundant. all the sched->ready is initialized as true, please >>> refer to drm_sched_init() >>> >> I see... so in the following commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)") >> that line is also redundant? > > Yes. > OK. > >>>> r = amdgpu_ring_test_ring(ring); >>>> if (r) { >>>> ring->sched.ready = false; >>>> @@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle) >>>> >>>> for (i = 0; i < adev->vcn.num_enc_rings; ++i) { >>>> ring = &adev->vcn.inst[j].ring_enc[i]; >>>> - ring->sched.ready = false; >>>> - continue; >>>> + ring->sched.ready = true; >>> Because the VCN 2.5 FW still has issue for encode, so we have it >>> disabled here. >>> >> OK. So, maybe we can add a comment pointing that out? > > That could be better. > Great. I'm glad it's not a bug. I'll write a patch for that so other people don't waste time taking a look. I appreciate your feedback. Thanks -- Gustavo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init 2019-10-01 21:57 ` Gustavo A. R. Silva (?) @ 2019-10-01 22:21 ` Liu, Leo 2019-10-02 13:32 ` Gustavo A. R. Silva -1 siblings, 1 reply; 10+ messages in thread From: Liu, Leo @ 2019-10-01 22:21 UTC (permalink / raw) To: Gustavo A. R. Silva, Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing), David Airlie, Daniel Vetter Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org On 2019-10-01 5:57 p.m., Gustavo A. R. Silva wrote: > > On 10/1/19 16:46, Liu, Leo wrote: > >>>>> + ring->sched.ready = true; >>>> This is redundant. all the sched->ready is initialized as true, please >>>> refer to drm_sched_init() >>>> >>> I see... so in the following commit 1b61de45dfaf ("drm/amdgpu: add initial VCN2.0 support (v2)") >>> that line is also redundant? >> Yes. >> > OK. > >>>>> r = amdgpu_ring_test_ring(ring); >>>>> if (r) { >>>>> ring->sched.ready = false; >>>>> @@ -266,8 +267,7 @@ static int vcn_v2_5_hw_init(void *handle) >>>>> >>>>> for (i = 0; i < adev->vcn.num_enc_rings; ++i) { >>>>> ring = &adev->vcn.inst[j].ring_enc[i]; >>>>> - ring->sched.ready = false; >>>>> - continue; >>>>> + ring->sched.ready = true; >>>> Because the VCN 2.5 FW still has issue for encode, so we have it >>>> disabled here. >>>> >>> OK. So, maybe we can add a comment pointing that out? >> That could be better. >> > Great. I'm glad it's not a bug. I'll write a patch for that so other > people don't waste time taking a look. Thanks, just sent two patches to add comment, and long with the patch to make VCN ring ready properly. Leo > > I appreciate your feedback. > Thanks > -- > Gustavo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init 2019-10-01 22:21 ` Liu, Leo @ 2019-10-02 13:32 ` Gustavo A. R. Silva 0 siblings, 0 replies; 10+ messages in thread From: Gustavo A. R. Silva @ 2019-10-02 13:32 UTC (permalink / raw) To: Liu, Leo, Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing), David Airlie, Daniel Vetter Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org On 10/1/19 17:21, Liu, Leo wrote: >>>> OK. So, maybe we can add a comment pointing that out? >>> That could be better. >>> >> Great. I'm glad it's not a bug. I'll write a patch for that so other >> people don't waste time taking a look. > > Thanks, just sent two patches to add comment, and long with the patch to > make VCN ring ready properly. > Awesome. Thank you. I would just add a commit log to this patch: [PATCH 2/2] drm/amdgpu: add a comment to VCN 2.5 encode ring I'd update the subject to: drm/amdgpu: add code comment in vcn_v2_5_hw_init and add this as a commit log: Add a comment to VCN 2.5 encode ring Also, I think it's important to follow the process and CC all the people and lists below: $ scripts/get_maintainer.pl --nokeywords --nogit --nogit-fallback drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c Alex Deucher <alexander.deucher@amd.com> (supporter:RADEON and AMDGPU DRM DRIVERS) "Christian König" <christian.koenig@amd.com> (supporter:RADEON and AMDGPU DRM DRIVERS) "David (ChunMing) Zhou" <David1.Zhou@amd.com> (supporter:RADEON and AMDGPU DRM DRIVERS) David Airlie <airlied@linux.ie> (maintainer:DRM DRIVERS) Daniel Vetter <daniel@ffwll.ch> (maintainer:DRM DRIVERS) amd-gfx@lists.freedesktop.org (open list:RADEON and AMDGPU DRM DRIVERS) dri-devel@lists.freedesktop.org (open list:DRM DRIVERS) linux-kernel@vger.kernel.org (open list) Thanks -- Gustavo ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-10-02 13:32 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-01 17:16 [PATCH] drm/amdgpu: fix structurally dead code vcn_v2_5_hw_init Gustavo A. R. Silva
2019-10-01 21:29 ` Liu, Leo
2019-10-01 21:29 ` Liu, Leo
2019-10-01 21:43 ` Gustavo A. R. Silva
[not found] ` <823b10a3-fe0e-2e8c-02c3-534944dbe6d2-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org>
2019-10-01 21:46 ` Liu, Leo
2019-10-01 21:46 ` Liu, Leo
2019-10-01 21:57 ` Gustavo A. R. Silva
2019-10-01 21:57 ` Gustavo A. R. Silva
2019-10-01 22:21 ` Liu, Leo
2019-10-02 13:32 ` Gustavo A. R. Silva
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.