missing wire up of nbif_v6_3_1_sriov_funcs ?

missing wire up of nbif_v6_3_1_sriov_funcs ?

[Dr. David Alan Gilbert]

Hi All,
  I think you may be misisng some wiring of nbif_v6_3_1_sriov_funcs.

  My scripts noticed 'nbif_v6_3_1_sriov_funcs' was unused;
It was added in:
  Commit: 894c6d3522d1 ("drm/amdgpu: Add nbif v6_3_1 ip block support")


and is:
drivers/gpu/drm/amd/amdgpu/nbif_v6_3_1.c:
  const struct amdgpu_nbio_funcs nbif_v6_3_1_sriov_funcs = {

but, it's not referenced, which makes me think it perhaps
should be in:
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c

I see for 4.3.x there is:
  case IP_VERSION(4, 3, 0):
  case IP_VERSION(4, 3, 1):
    if (amdgpu_sriov_vf(adev))
      adev->nbio.funcs = &nbio_v4_3_sriov_funcs;
    else
      adev->nbio.funcs = &nbio_v4_3_funcs;
    adev->nbio.hdp_flush_reg = &nbio_v4_3_hdp_flush_reg;
    break;

but for 6.3.1 we have:

  case IP_VERSION(6, 3, 1):
    adev->nbio.funcs = &nbif_v6_3_1_funcs;
    adev->nbio.hdp_flush_reg = &nbif_v6_3_1_hdp_flush_reg;
    break;


so is that supposed to have something similar?

Dave

-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: missing wire up of nbif_v6_3_1_sriov_funcs ?

[Alex Deucher]

On Wed, Feb 19, 2025 at 2:04 PM Dr. David Alan Gilbert
<linux@treblig.org> wrote:
>
> Hi All,
>   I think you may be misisng some wiring of nbif_v6_3_1_sriov_funcs.
>
>   My scripts noticed 'nbif_v6_3_1_sriov_funcs' was unused;
> It was added in:
>   Commit: 894c6d3522d1 ("drm/amdgpu: Add nbif v6_3_1 ip block support")
>
>
> and is:
> drivers/gpu/drm/amd/amdgpu/nbif_v6_3_1.c:
>   const struct amdgpu_nbio_funcs nbif_v6_3_1_sriov_funcs = {
>
> but, it's not referenced, which makes me think it perhaps
> should be in:
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
>
> I see for 4.3.x there is:
>   case IP_VERSION(4, 3, 0):
>   case IP_VERSION(4, 3, 1):
>     if (amdgpu_sriov_vf(adev))
>       adev->nbio.funcs = &nbio_v4_3_sriov_funcs;
>     else
>       adev->nbio.funcs = &nbio_v4_3_funcs;
>     adev->nbio.hdp_flush_reg = &nbio_v4_3_hdp_flush_reg;
>     break;
>
> but for 6.3.1 we have:
>
>   case IP_VERSION(6, 3, 1):
>     adev->nbio.funcs = &nbif_v6_3_1_funcs;
>     adev->nbio.hdp_flush_reg = &nbif_v6_3_1_hdp_flush_reg;
>     break;
>
>
> so is that supposed to have something similar?

They can be dropped.  I think they were just copy and paste leftovers.

Alex

>
> Dave
>
> --
>  -----Open up your eyes, open up your mind, open up your code -------
> / Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \
> \        dave @ treblig.org |                               | In Hex /
>  \ _________________________|_____ http://www.treblig.org   |_______/

Re: missing wire up of nbif_v6_3_1_sriov_funcs ?

[Dr. David Alan Gilbert]

* Alex Deucher (alexdeucher@gmail.com) wrote:
> On Wed, Feb 19, 2025 at 2:04 PM Dr. David Alan Gilbert
> <linux@treblig.org> wrote:
> >
> > Hi All,
> >   I think you may be misisng some wiring of nbif_v6_3_1_sriov_funcs.
> >
> >   My scripts noticed 'nbif_v6_3_1_sriov_funcs' was unused;
> > It was added in:
> >   Commit: 894c6d3522d1 ("drm/amdgpu: Add nbif v6_3_1 ip block support")
> >
> >
> > and is:
> > drivers/gpu/drm/amd/amdgpu/nbif_v6_3_1.c:
> >   const struct amdgpu_nbio_funcs nbif_v6_3_1_sriov_funcs = {
> >
> > but, it's not referenced, which makes me think it perhaps
> > should be in:
> > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> >
> > I see for 4.3.x there is:
> >   case IP_VERSION(4, 3, 0):
> >   case IP_VERSION(4, 3, 1):
> >     if (amdgpu_sriov_vf(adev))
> >       adev->nbio.funcs = &nbio_v4_3_sriov_funcs;
> >     else
> >       adev->nbio.funcs = &nbio_v4_3_funcs;
> >     adev->nbio.hdp_flush_reg = &nbio_v4_3_hdp_flush_reg;
> >     break;
> >
> > but for 6.3.1 we have:
> >
> >   case IP_VERSION(6, 3, 1):
> >     adev->nbio.funcs = &nbif_v6_3_1_funcs;
> >     adev->nbio.hdp_flush_reg = &nbif_v6_3_1_hdp_flush_reg;
> >     break;
> >
> >
> > so is that supposed to have something similar?
> 
> They can be dropped.  I think they were just copy and paste leftovers.

Thanks for the reply.

OK, I'll send a patch then to remove nbif_v6_3_1_sriov_funcs.

Dave

> Alex
> 
> >
> > Dave
> >
> > --
> >  -----Open up your eyes, open up your mind, open up your code -------
> > / Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \
> > \        dave @ treblig.org |                               | In Hex /
> >  \ _________________________|_____ http://www.treblig.org   |_______/
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: missing wire up of nbif_v6_3_1_sriov_funcs ?

[Alex Deucher]

On Wed, Feb 19, 2025 at 3:00 PM Dr. David Alan Gilbert <dave@treblig.org> wrote:
>
> * Alex Deucher (alexdeucher@gmail.com) wrote:
> > On Wed, Feb 19, 2025 at 2:04 PM Dr. David Alan Gilbert
> > <linux@treblig.org> wrote:
> > >
> > > Hi All,
> > >   I think you may be misisng some wiring of nbif_v6_3_1_sriov_funcs.
> > >
> > >   My scripts noticed 'nbif_v6_3_1_sriov_funcs' was unused;
> > > It was added in:
> > >   Commit: 894c6d3522d1 ("drm/amdgpu: Add nbif v6_3_1 ip block support")
> > >
> > >
> > > and is:
> > > drivers/gpu/drm/amd/amdgpu/nbif_v6_3_1.c:
> > >   const struct amdgpu_nbio_funcs nbif_v6_3_1_sriov_funcs = {
> > >
> > > but, it's not referenced, which makes me think it perhaps
> > > should be in:
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
> > >
> > > I see for 4.3.x there is:
> > >   case IP_VERSION(4, 3, 0):
> > >   case IP_VERSION(4, 3, 1):
> > >     if (amdgpu_sriov_vf(adev))
> > >       adev->nbio.funcs = &nbio_v4_3_sriov_funcs;
> > >     else
> > >       adev->nbio.funcs = &nbio_v4_3_funcs;
> > >     adev->nbio.hdp_flush_reg = &nbio_v4_3_hdp_flush_reg;
> > >     break;
> > >
> > > but for 6.3.1 we have:
> > >
> > >   case IP_VERSION(6, 3, 1):
> > >     adev->nbio.funcs = &nbif_v6_3_1_funcs;
> > >     adev->nbio.hdp_flush_reg = &nbif_v6_3_1_hdp_flush_reg;
> > >     break;
> > >
> > >
> > > so is that supposed to have something similar?
> >
> > They can be dropped.  I think they were just copy and paste leftovers.
>
> Thanks for the reply.
>
> OK, I'll send a patch then to remove nbif_v6_3_1_sriov_funcs.

Thanks!

Alex

>
> Dave
>
> > Alex
> >
> > >
> > > Dave
> > >
> > > --
> > >  -----Open up your eyes, open up your mind, open up your code -------
> > > / Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \
> > > \        dave @ treblig.org |                               | In Hex /
> > >  \ _________________________|_____ http://www.treblig.org   |_______/
> --
>  -----Open up your eyes, open up your mind, open up your code -------
> / Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \
> \        dave @ treblig.org |                               | In Hex /
>  \ _________________________|_____ http://www.treblig.org   |_______/