Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i787e41f1:Fastmail
Date:   Mon, 15 Aug 2022 18:12:06 +0200
From:   Greg KH <greg@kroah.com>
To:     Khalid Masum <khalid.masum.92@gmail.com>
Cc:     "Dong, Ruijing" <Ruijing.Dong@amd.com>,
        "amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-kernel-mentees@lists.linuxfoundation.org" 
        <linux-kernel-mentees@lists.linuxfoundation.org>,
        Wan Jiabing <wanjiabing@vivo.com>,
        David Airlie <airlied@linux.ie>,
        "Pan, Xinhui" <Xinhui.Pan@amd.com>,
        "Jiang, Sonny" <Sonny.Jiang@amd.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        "Deucher, Alexander" <Alexander.Deucher@amd.com>,
        "Zhu, James" <James.Zhu@amd.com>, "Liu, Leo" <Leo.Liu@amd.com>,
        "Koenig, Christian" <Christian.Koenig@amd.com>
Subject: Re: [PATCH linux-next] drm/amdgpu/vcn: Remove unused assignment in
 vcn_v4_0_stop
Message-ID: <YvpwVh359EMGa5kO@kroah.com>
References: <20220815070056.10816-1-khalid.masum.92@gmail.com>
 <SJ1PR12MB61944B1D53330D5E9531158695689@SJ1PR12MB6194.namprd12.prod.outlook.com>
 <86088c17-585c-4a53-312d-ef339b824538@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <86088c17-585c-4a53-312d-ef339b824538@gmail.com>
Precedence: bulk

On Mon, Aug 15, 2022 at 09:11:18PM +0600, Khalid Masum wrote:
> On 8/15/22 20:15, Dong, Ruijing wrote:
> > [AMD Official Use Only - General]
> > 
> > Sorry, which "r" value was overwritten?  I didn't see the point of making this change.
> > 
> > Thanks
> > Ruijing
> > 
> > -----Original Message-----
> > From: Khalid Masum <khalid.masum.92@gmail.com>
> > Sent: Monday, August 15, 2022 3:01 AM
> > To: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org; linux-kernel-mentees@lists.linuxfoundation.org
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; David Airlie <airlied@linux.ie>; Daniel Vetter <daniel@ffwll.ch>; Zhu, James <James.Zhu@amd.com>; Jiang, Sonny <Sonny.Jiang@amd.com>; Dong, Ruijing <Ruijing.Dong@amd.com>; Wan Jiabing <wanjiabing@vivo.com>; Liu, Leo <Leo.Liu@amd.com>; Khalid Masum <khalid.masum.92@gmail.com>
> > Subject: [PATCH linux-next] drm/amdgpu/vcn: Remove unused assignment in vcn_v4_0_stop
> > 
> > The value assigned from vcn_v4_0_stop_dbg_mode to r is overwritten before it can be used. Remove this assignment.
> > 
> > Addresses-Coverity: 1504988 ("Unused value")
> > Fixes: 8da1170a16e4 ("drm/amdgpu: add VCN4 ip block support")
> > Signed-off-by: Khalid Masum <khalid.masum.92@gmail.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> > index ca14c3ef742e..80b8a2c66b36 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> > @@ -1154,7 +1154,7 @@ static int vcn_v4_0_stop(struct amdgpu_device *adev)
> >                  fw_shared->sq.queue_mode |= FW_QUEUE_DPG_HOLD_OFF;
> > 
> >                  if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> > -                       r = vcn_v4_0_stop_dpg_mode(adev, i);
> > +                       vcn_v4_0_stop_dpg_mode(adev, i);
> >                          continue;
> >                  }
> > 
> > --
> > 2.37.1
> > 
> 
> After value is overwritten soon right after the diff.
> 
> See:
> drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> 
> static int vcn_v4_0_stop(struct amdgpu_device *adev)
> {
>         volatile struct amdgpu_vcn4_fw_shared *fw_shared;
> ...
> 
>         for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
>                 fw_shared = adev->vcn.inst[i].fw_shared.cpu_addr;
>                 fw_shared->sq.queue_mode |= FW_QUEUE_DPG_HOLD_OFF;
> 
>                 if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
>                         r = vcn_v4_0_stop_dpg_mode(adev, i);
>                         continue;
>                 }
> 
>                 /* wait for vcn idle */
>                 r = SOC15_WAIT_ON_RREG(VCN, i, regUVD_STATUS,
> UVD_STATUS__IDLE, 0x7);
> 
> Here, any value assigned to r is overwritten before it could
> be used. So the assignment in the true branch of the if statement
> here can be removed.

Why not fix vcn_v4_0_stop_dpg_mode() to not return anything, as it does
not, and then remove this assignment as well, which would fix up
everything at once to be more obvious what is happening and why.

thanks,

greg k-h