[PATCH] drm/amdgpu: Save VCN shared memory with init reset

[Liu, Leo]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar at amd.com>
> Sent: October 17, 2024 9:17 AM
> To: Liu, Leo <Leo.Liu at amd.com>; Koenig, Christian
> <Christian.Koenig at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Zhang, Hawking <Hawking.Zhang at amd.com>; Deucher, Alexander
> <Alexander.Deucher at amd.com>; Sundararaju, Sathishkumar
> <Sathishkumar.Sundararaju at amd.com>; Jiang, Sonny
> <Sonny.Jiang at amd.com>; Zhou, Hao (Claire) <Hao.Zhou at amd.com>
> Subject: Re: [PATCH] drm/amdgpu: Save VCN shared memory with init reset
>
>
>
> On 10/17/2024 6:37 PM, Liu, Leo wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> >
> >> -----Original Message-----
> >> From: Lazar, Lijo <Lijo.Lazar at amd.com>
> >> Sent: October 16, 2024 11:18 PM
> >> To: Liu, Leo <Leo.Liu at amd.com>; Koenig, Christian
> >> <Christian.Koenig at amd.com>; amd-gfx at lists.freedesktop.org
> >> Cc: Zhang, Hawking <Hawking.Zhang at amd.com>; Deucher, Alexander
> >> <Alexander.Deucher at amd.com>; Sundararaju, Sathishkumar
> >> <Sathishkumar.Sundararaju at amd.com>; Jiang, Sonny
> >> <Sonny.Jiang at amd.com>; Zhou, Hao (Claire) <Hao.Zhou at amd.com>
> >> Subject: Re: [PATCH] drm/amdgpu: Save VCN shared memory with init
> >> reset
> >>
> >>
> >>
> >> On 10/16/2024 9:37 PM, Liu, Leo wrote:
> >>> [AMD Official Use Only - AMD Internal Distribution Only]
> >>>
> >>>> -----Original Message-----
> >>>> From: Koenig, Christian <Christian.Koenig at amd.com>
> >>>> Sent: October 16, 2024 9:16 AM
> >>>> To: Lazar, Lijo <Lijo.Lazar at amd.com>;
> >>>> amd-gfx at lists.freedesktop.org; Liu, Leo <Leo.Liu at amd.com>
> >>>> Cc: Zhang, Hawking <Hawking.Zhang at amd.com>; Deucher, Alexander
> >>>> <Alexander.Deucher at amd.com>; Sundararaju, Sathishkumar
> >>>> <Sathishkumar.Sundararaju at amd.com>; Jiang, Sonny
> >>>> <Sonny.Jiang at amd.com>; Zhou, Hao (Claire) <Hao.Zhou at amd.com>
> >>>> Subject: Re: [PATCH] drm/amdgpu: Save VCN shared memory with init
> >>>> reset
> >>>>
> >>>> Am 15.10.24 um 08:23 schrieb Lijo Lazar:
> >>>>> VCN shared memory is in framebuffer and there are some flags
> >>>>> initialized during sw_init. Ideally, such programming should be
> >>>>> during
> >>>> hw_init.
> >>>>
> >>>> IIRC that was intentionally not done during hw_init for some reason.
> >>>> @Leo do you remember why?
> >>>>
> >>>
> >>> We need to keep some of the status from share memory(driver and FW),
> >> since some of them are changed by FW, that is why the init cannot be
> >> in the hw_init stage with suspend/resume case.
> >>>
> >>
> >> For vcn_v4_0_3, the flags that are initialized in sw_init are these
> >>
> >>                 fw_shared->present_flag_0 =
> >> cpu_to_le32(AMDGPU_FW_SHARED_FLAG_0_UNIFIED_QUEUE);
> >>                 fw_shared->sq.is_enabled = true;
> >>
> >>                 if (amdgpu_vcnfw_log)
> >>                         amdgpu_vcn_fwlog_init(&adev->vcn.inst[i]);
> >>
> >> Is the flags initialized during sw_init required for FW during its
> >> initialization stage? If not, it would be better to move this to hw_init.
> >>
> >> Some part also get modified during vcn_v4_0_3_start (which is after
> >> hw_init during runtime) -
> >>
> >>                 fw_shared->sq.queue_mode &=
> >>                         cpu_to_le32(~(FW_QUEUE_RING_RESET |
> >> FW_QUEUE_DPG_HOLD_OFF));
> >>
> >>
> >> One reason probably is hw_init is also called during resume which
> >> restores the saved bo during suspend. So this may be to avoid the double
> work.
> >>
> >> Anyway, is the patch okay to go?
> >>
> > You need to fix the function name as I commented from last email.
> >
>
> Any suggestions - amdgpu_vcn_save_vcpu_buffer/bo?
>
"amdgpu_vcn_save_vcpu_bo" should be good.

Leo

> Thanks,
> Lijo
>
> > Regards,
> > Leo
> >
> >
> >> Thanks,
> >> Lijo
> >>
> >>>  +int amdgpu_vcn_suspend(struct amdgpu_device *adev) {
> >>>  > +    bool in_ras_intr = amdgpu_ras_intr_triggered();
> >>>  > +
> >>>  > +    cancel_delayed_work_sync(&adev->vcn.idle_work);
> >>>  > +
> >>>  > +    /* err_event_athub will corrupt VCPU buffer, so we need to
> >>>  > +     * restore fw data and clear buffer in amdgpu_vcn_resume() */
> >>>  > +    if (in_ras_intr)
> >>>  > +            return 0;
> >>>  > +
> >>>  > +    return amdgpu_vcn_save_fw_shared_region(adev);
> >>> The saved bo here is not only for fw shared memory, also for FW
> >>> runtime
> >> stack/heap as well.
> >>>> Regards,
> >>> Leo
> >>>
> >>>  > +}
> >>>  > +