On Wed, Aug 20, 2025 at 6:09 PM Rodrigo Siqueira <sique...@igalia.com> wrote:
>
> On 08/18, Alex Deucher wrote:
> > On Sat, Aug 16, 2025 at 12:28 PM Rodrigo Siqueira <sique...@igalia.com> 
> > wrote:
> > >
> > > In the loop that iterates over the different VCN instances from VCN
> > > 4.0.3, the same irq source has been passed for different instances.
> > > This commit addresses the issue by adding the missing index to the array
> > > access for the IRQ.
> >
> > This is on purpose.  There are no per instance source ids on 4.0.3.
> > The IH packets on this chip have a separate field to differentiate the
> > instances.
>
> Thanks for the explanation. Is this rationale also valid for VCN 1.0 and
> VCN 2.0?

VCN 1.0 and 2.0 only exist as single instances.  There are no chips
with multiple instances of them.

>
> Also, do you think it is worth making this hardware difference more
> evident in the code with something like this:
>
> -               r = amdgpu_ring_init(adev, ring, 512, &adev->vcn.inst[i].irq, 
> 0,
> +
> +               // There are no per-instance source IDs on 4.0.3, the IH
> +               // packets use a separate field to differentiate instances.

If you think it's helpful.  You may want to clarify that this comment
refers to the irqs ("per-instance irq source IDs").  Also, please use
C style comments.

Alex

> +               r = amdgpu_ring_init(adev, ring, 512, &adev->vcn.inst[0].irq, 
> 0,
>
> Thanks
>
> >
> > Alex
> >
> > >
> > > Signed-off-by: Rodrigo Siqueira <sique...@igalia.com>
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c 
> > > b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
> > > index a63a1e3435ab..018a526a8801 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
> > > @@ -212,7 +212,7 @@ static int vcn_v4_0_3_sw_init(struct amdgpu_ip_block 
> > > *ip_block)
> > >
> > >                 ring->vm_hub = AMDGPU_MMHUB0(adev->vcn.inst[i].aid_id);
> > >                 sprintf(ring->name, "vcn_unified_%d", 
> > > adev->vcn.inst[i].aid_id);
> > > -               r = amdgpu_ring_init(adev, ring, 512, 
> > > &adev->vcn.inst->irq, 0,
> > > +               r = amdgpu_ring_init(adev, ring, 512, 
> > > &adev->vcn.inst[i].irq, 0,
> > >                                      AMDGPU_RING_PRIO_DEFAULT,
> > >                                      &adev->vcn.inst[i].sched_score);
> > >                 if (r)
> > > --
> > > 2.47.2
> > >
>
> --
> Rodrigo Siqueira

Reply via email to