Re: [PATCH] drm/amdgpu: fix limiting AV1 to the first instance on VCN3

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

I need to look into this more deeply when I'm back from sick leave.

Till then this workaround should be sufficient since VCN3 is the only 
callback which tries to adjust the instance.

Regards,
Christian.

Am 07.06.22 um 22:22 schrieb Alex Deucher:
> We'll need to implement the parse callbacks for vcn4 as well if we
> haven't already.
>
> Alex
>
> On Tue, Jun 7, 2022 at 4:20 PM Dong, Ruijing <Ruijing.Dong@xxxxxxx> wrote:
> > [AMD Official Use Only - General]
> >
> > I can see for VCN4, AV1 dec/enc also need to limit to the first instance.
> >
> > Thanks,
> > Ruijing
> >
> > -----Original Message-----
> > From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Alex Deucher
> > Sent: Friday, June 3, 2022 10:12 AM
> > To: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
> > Cc: Pelloux-Prayer, Pierre-Eric <Pierre-eric.Pelloux-prayer@xxxxxxx>; amd-gfx list <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
> > Subject: Re: [PATCH] drm/amdgpu: fix limiting AV1 to the first instance on VCN3
> >
> > Do the other uvd/vce/vcn ring parse functions need a similar fix?
> >
> > Alex
> >
> >
> > On Fri, Jun 3, 2022 at 10:08 AM Alex Deucher <alexdeucher@xxxxxxxxx> wrote:
> > > On Fri, Jun 3, 2022 at 8:10 AM Christian König
> > > <ckoenig.leichtzumerken@xxxxxxxxx> wrote:
> > > > Am 03.06.22 um 14:08 schrieb Pierre-Eric Pelloux-Prayer:
> > > > > Hi Christian,
> > > > >
> > > > > The patch is: Tested-by: Pierre-Eric Pelloux-Prayer
> > > > > <pierre-eric.pelloux-prayer@xxxxxxx>
> > > > >
> > > > > Could you add a reference to https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F2037&amp;data=05%7C01%7CRuijing.Dong%40amd.com%7C5ba73dfe91ba47e21dd608da456b0609%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637898623221806051%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=WgIZD299Xe0XVG%2Ftb2rn14njS%2FgHIhtIHIDAZ2Fj40k%3D&amp;reserved=0 in the commit message?
> > > > Sure, can you also give me an rb or acked-by so that I can push it?
> > > Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
> > >
> > > > Thanks,
> > > > Christian.
> > > >
> > > > > Thanks,
> > > > > Pierre-Eric
> > > > >
> > > > > On 03/06/2022 12:21, Christian König wrote:
> > > > > > The job is not yet initialized here.
> > > > > >
> > > > > > Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> > > > > > Fixes: 1027d5d791b7 ("drm/amdgpu: use job and ib structures
> > > > > > directly in CS parsers")
> > > > > > ---
> > > > > >    drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 17 +++++++----------
> > > > > >    1 file changed, 7 insertions(+), 10 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > > > > > b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > > > > > index 3cabceee5f57..39405f0db824 100644
> > > > > > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > > > > > @@ -1761,23 +1761,21 @@ static const struct amdgpu_ring_funcs vcn_v3_0_dec_sw_ring_vm_funcs = {
> > > > > >       .emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
> > > > > >    };
> > > > > >
> > > > > > -static int vcn_v3_0_limit_sched(struct amdgpu_cs_parser *p,
> > > > > > -                            struct amdgpu_job *job)
> > > > > > +static int vcn_v3_0_limit_sched(struct amdgpu_cs_parser *p)
> > > > > >    {
> > > > > >       struct drm_gpu_scheduler **scheds;
> > > > > >
> > > > > >       /* The create msg must be in the first IB submitted */
> > > > > > -    if (atomic_read(&job->base.entity->fence_seq))
> > > > > > +    if (atomic_read(&p->entity->fence_seq))
> > > > > >               return -EINVAL;
> > > > > >
> > > > > >       scheds = p->adev->gpu_sched[AMDGPU_HW_IP_VCN_DEC]
> > > > > >               [AMDGPU_RING_PRIO_DEFAULT].sched;
> > > > > > -    drm_sched_entity_modify_sched(job->base.entity, scheds, 1);
> > > > > > +    drm_sched_entity_modify_sched(p->entity, scheds, 1);
> > > > > >       return 0;
> > > > > >    }
> > > > > >
> > > > > > -static int vcn_v3_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
> > > > > > -                        uint64_t addr)
> > > > > > +static int vcn_v3_0_dec_msg(struct amdgpu_cs_parser *p, uint64_t
> > > > > > +addr)
> > > > > >    {
> > > > > >       struct ttm_operation_ctx ctx = { false, false };
> > > > > >       struct amdgpu_bo_va_mapping *map; @@ -1848,7 +1846,7 @@
> > > > > > static int vcn_v3_0_dec_msg(struct amdgpu_cs_parser *p, struct amdgpu_job *job,
> > > > > >               if (create[0] == 0x7 || create[0] == 0x10 || create[0] == 0x11)
> > > > > >                       continue;
> > > > > >
> > > > > > -            r = vcn_v3_0_limit_sched(p, job);
> > > > > > +            r = vcn_v3_0_limit_sched(p);
> > > > > >               if (r)
> > > > > >                       goto out;
> > > > > >       }
> > > > > > @@ -1862,7 +1860,7 @@ static int vcn_v3_0_ring_patch_cs_in_place(struct amdgpu_cs_parser *p,
> > > > > >                                          struct amdgpu_job *job,
> > > > > >                                          struct amdgpu_ib *ib)
> > > > > >    {
> > > > > > -    struct amdgpu_ring *ring = to_amdgpu_ring(job->base.sched);
> > > > > > +    struct amdgpu_ring *ring =
> > > > > > + to_amdgpu_ring(p->entity->rq->sched);
> > > > > >       uint32_t msg_lo = 0, msg_hi = 0;
> > > > > >       unsigned i;
> > > > > >       int r;
> > > > > > @@ -1881,8 +1879,7 @@ static int vcn_v3_0_ring_patch_cs_in_place(struct amdgpu_cs_parser *p,
> > > > > >                       msg_hi = val;
> > > > > >               } else if (reg == PACKET0(p->adev->vcn.internal.cmd, 0) &&
> > > > > >                          val == 0) {
> > > > > > -                    r = vcn_v3_0_dec_msg(p, job,
> > > > > > -                                         ((u64)msg_hi) << 32 | msg_lo);
> > > > > > +                    r = vcn_v3_0_dec_msg(p, ((u64)msg_hi) << 32
> > > > > > + | msg_lo);
> > > > > >                       if (r)
> > > > > >                               return r;
> > > > > >               }