[PATCH] drm/amdgpu: fix some uninitialized variables

Alex Deucher alexdeucher at gmail.com
Wed Apr 24 02:34:52 UTC 2024


Fix Leo's address.

On Tue, Apr 23, 2024 at 10:33 PM Alex Deucher <alexdeucher at gmail.com> wrote:
>
> On Tue, Apr 23, 2024 at 10:04 PM Zhang, Jesse(Jie) <Jesse.Zhang at amd.com> wrote:
> >
> > [AMD Official Use Only - General]
> >
> > Hi Alex,
> >
> > -----Original Message-----
> > From: Alex Deucher <alexdeucher at gmail.com>
> > Sent: Wednesday, April 24, 2024 9:46 AM
> > To: Zhang, Jesse(Jie) <Jesse.Zhang at amd.com>
> > Cc: amd-gfx at lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>; Huang, Tim <Tim.Huang at amd.com>
> > Subject: Re: [PATCH] drm/amdgpu: fix some uninitialized variables
> >
> > On Tue, Apr 23, 2024 at 9:27 PM Jesse Zhang <jesse.zhang at amd.com> wrote:
> > >
> > > Fix some variables not initialized before use.
> > > Scan them out using Synopsys tools.
> > >
> > > Signed-off-by: Jesse Zhang <Jesse.Zhang at amd.com>
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 2 +-
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 5 +++++
> > >  drivers/gpu/drm/amd/amdgpu/atom.c       | 1 +
> > >  drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c  | 3 ++-
> > > drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c  | 3 ++-
> > > drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c  | 3 ++-
> >
> > Please split out the SDMA changes into a separate patch.
> >
> > More comments below on the other hunks.
> >
> > >  6 files changed, 13 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> > > index 59acf424a078..60d97cd14855 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> > > @@ -742,7 +742,7 @@ int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p,
> > >         uint32_t destroyed = 0;
> > >         uint32_t created = 0;
> > >         uint32_t allocated = 0;
> > > -       uint32_t tmp, handle = 0;
> > > +       uint32_t tmp = 0, handle = 0;
> >
> > Can you elaborate on what the issue is here?  Presumably it's warning about size being passed as a parameter in the function?
> > [Zhang, Jesse(Jie)]  Using uninitialized value *size when calling amdgpu_vce_cs_reloc for case 0x03000001. Because uint32_t *size = &tmp;
> >                 I'm not sure if other commands initialize the size before running case 0x03000001.
>
> Yeah, I don't really see a cleaner way to handle this.  Maybe Leo has
> a better idea?
>
> Alex
>
> >
> > >         uint32_t *size = &tmp;
> > >         unsigned int idx;
> > >         int i, r = 0;
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> > > index 677eb141554e..13125ddd5e86 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> > > @@ -410,6 +410,11 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct *work)
> > >                         else
> > >                                 new_state.fw_based =
> > > VCN_DPG_STATE__UNPAUSE;
> > >
> > > +                       if (amdgpu_fence_count_emitted(adev->jpeg.inst->ring_dec))
> > > +                               new_state.jpeg = VCN_DPG_STATE__PAUSE;
> > > +                       else
> > > +                               new_state.jpeg =
> > > + VCN_DPG_STATE__UNPAUSE;
> > > +
> > >                         adev->vcn.pause_dpg_mode(adev, j, &new_state);
> > >                 }
> > >
> >
> > This should be a separate patch as well.
> >  Thanks for your reminder, Alex, I will.
> >
> >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c
> > > b/drivers/gpu/drm/amd/amdgpu/atom.c
> > > index 72362df352f6..d552e013354c 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/atom.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/atom.c
> > > @@ -1243,6 +1243,7 @@ static int amdgpu_atom_execute_table_locked(struct atom_context *ctx, int index,
> > >         ectx.ps_size = params_size;
> > >         ectx.abort = false;
> > >         ectx.last_jump = 0;
> > > +       ectx.last_jump_jiffies = 0;
> > >         if (ws) {
> > >                 ectx.ws = kcalloc(4, ws, GFP_KERNEL);
> > >                 ectx.ws_size = ws;
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > > b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > > index 45a2d0a5a2d7..b7d33d78bce0 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > > @@ -999,7 +999,8 @@ static int sdma_v5_0_ring_test_ring(struct amdgpu_ring *ring)
> > >         r = amdgpu_ring_alloc(ring, 20);
> > >         if (r) {
> > >                 DRM_ERROR("amdgpu: dma failed to lock ring %d (%d).\n", ring->idx, r);
> > > -               amdgpu_device_wb_free(adev, index);
> > > +               if (!ring->is_mes_queue)
> > > +                       amdgpu_device_wb_free(adev, index);
> > >                 return r;
> > >         }
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > index 43e64b2da575..cc9e961f0078 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > @@ -839,7 +839,8 @@ static int sdma_v5_2_ring_test_ring(struct amdgpu_ring *ring)
> > >         r = amdgpu_ring_alloc(ring, 20);
> > >         if (r) {
> > >                 DRM_ERROR("amdgpu: dma failed to lock ring %d (%d).\n", ring->idx, r);
> > > -               amdgpu_device_wb_free(adev, index);
> > > +               if (!ring->is_mes_queue)
> > > +                       amdgpu_device_wb_free(adev, index);
> > >                 return r;
> > >         }
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> > > b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> > > index 1f4877195213..c833b6b8373b 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> > > @@ -861,7 +861,8 @@ static int sdma_v6_0_ring_test_ring(struct amdgpu_ring *ring)
> > >         r = amdgpu_ring_alloc(ring, 5);
> > >         if (r) {
> > >                 DRM_ERROR("amdgpu: dma failed to lock ring %d (%d).\n", ring->idx, r);
> > > -               amdgpu_device_wb_free(adev, index);
> > > +               if (!ring->is_mes_queue)
> > > +                       amdgpu_device_wb_free(adev, index);
> > >                 return r;
> > >         }
> > >
> > > --
> > > 2.25.1
> > >


More information about the amd-gfx mailing list