All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lang Yu <Lang.Yu@amd.com>
To: "Lazar, Lijo" <lijo.lazar@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: use psp_execute_load_ip_fw_cmd_buf instead
Date: Fri, 7 Jul 2023 19:11:54 +0800	[thread overview]
Message-ID: <ZKfy+kffRgVnsXZR@lang-desktop> (raw)
In-Reply-To: <d861bfc9-f424-1fc3-788c-73d103d333c9@amd.com>

On 07/07/ , Lazar, Lijo wrote:
> 
> 
> On 6/29/2023 1:44 PM, Lang Yu wrote:
> > Ping.
> > 
> > On 06/27/ , Lang Yu wrote:
> > > Replace the old ones with psp_execute_load_ip_fw_cmd_buf.
> > > 
> > > Signed-off-by: Lang Yu <Lang.Yu@amd.com>
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 31 ++++---------------------
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 --
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c |  9 +++++++
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  2 ++
> > >   drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c   |  4 +---
> > >   drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c   |  4 +---
> > >   drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c   |  4 +---
> > >   drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c   |  4 +---
> > >   drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c |  4 +---
> > >   9 files changed, 20 insertions(+), 44 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > index a1cb541f315f..b61963112118 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > @@ -2474,21 +2474,11 @@ int psp_execute_load_ip_fw_cmd_buf(struct amdgpu_device *adev,
> > >   	return ret;
> > >   }
> > > -static int psp_execute_non_psp_fw_load(struct psp_context *psp,
> > > -				  struct amdgpu_firmware_info *ucode)
> > > +static inline
> > > +int psp_execute_non_psp_fw_load(struct psp_context *psp,
> > > +				struct amdgpu_firmware_info *ucode)
> > >   {
> > > -	int ret = 0;
> > > -	struct psp_gfx_cmd_resp *cmd = acquire_psp_cmd_buf(psp);
> > > -
> > > -	ret = psp_prep_load_ip_fw_cmd_buf(ucode, cmd);
> > > -	if (!ret) {
> > > -		ret = psp_cmd_submit_buf(psp, ucode, cmd,
> > > -					 psp->fence_buf_mc_addr);
> > > -	}
> > > -
> > > -	release_psp_cmd_buf(psp);
> > > -
> > > -	return ret;
> > > +	return psp_execute_load_ip_fw_cmd_buf(psp->adev, ucode, 0, 0, 0);
> > >   }
> > >   static int psp_load_smu_fw(struct psp_context *psp)
> > > @@ -2946,19 +2936,6 @@ int psp_rlc_autoload_start(struct psp_context *psp)
> > >   	return ret;
> > >   }
> > > -int psp_update_vcn_sram(struct amdgpu_device *adev, int inst_idx,
> > > -			uint64_t cmd_gpu_addr, int cmd_size)
> > > -{
> > > -	struct amdgpu_firmware_info ucode = {0};
> > > -
> > > -	ucode.ucode_id = inst_idx ? AMDGPU_UCODE_ID_VCN1_RAM :
> > > -		AMDGPU_UCODE_ID_VCN0_RAM;
> > > -	ucode.mc_addr = cmd_gpu_addr;
> > > -	ucode.ucode_size = cmd_size;
> > > -
> > > -	return psp_execute_non_psp_fw_load(&adev->psp, &ucode);
> > > -}
> > > -
> > >   int psp_ring_cmd_submit(struct psp_context *psp,
> > >   			uint64_t cmd_buf_mc_addr,
> > >   			uint64_t fence_mc_addr,
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> > > index bd324fed6237..e49984a9d570 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> > > @@ -459,8 +459,6 @@ extern int psp_wait_for_spirom_update(struct psp_context *psp, uint32_t reg_inde
> > >   			uint32_t field_val, uint32_t mask, uint32_t msec_timeout);
> > >   int psp_gpu_reset(struct amdgpu_device *adev);
> > > -int psp_update_vcn_sram(struct amdgpu_device *adev, int inst_idx,
> > > -			uint64_t cmd_gpu_addr, int cmd_size);
> > >   int psp_execute_load_ip_fw_cmd_buf(struct amdgpu_device *adev,
> > >   				   struct amdgpu_firmware_info *ucode,
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> > > index d37ebd4402ef..1805cd042d34 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> > > @@ -1257,3 +1257,12 @@ int amdgpu_vcn_ras_sw_init(struct amdgpu_device *adev)
> > >   	return 0;
> > >   }
> > > +
> > > +int amdgpu_vcn_psp_update_sram(struct amdgpu_device *adev, int inst_idx)
> > > +{
> > > +	return psp_execute_load_ip_fw_cmd_buf(adev, NULL,
> > > +		inst_idx ? AMDGPU_UCODE_ID_VCN1_RAM : AMDGPU_UCODE_ID_VCN0_RAM,
> > > +		adev->vcn.inst[inst_idx].dpg_sram_gpu_addr,
> > > +		(uint32_t)((uintptr_t)adev->vcn.inst[inst_idx].dpg_sram_curr_addr -
> > > +			   (uintptr_t)adev->vcn.inst[inst_idx].dpg_sram_cpu_addr));
> > > +}
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> > > index 92d5534df5f4..3ac5ad91ed08 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> > > @@ -414,4 +414,6 @@ int amdgpu_vcn_ras_late_init(struct amdgpu_device *adev,
> > >   			struct ras_common_if *ras_block);
> > >   int amdgpu_vcn_ras_sw_init(struct amdgpu_device *adev);
> > > +int amdgpu_vcn_psp_update_sram(struct amdgpu_device *adev, int inst_idx);
> > > +
> > >   #endif
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> > > index c975aed2f6c7..74cd1522067c 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> > > @@ -881,9 +881,7 @@ static int vcn_v2_0_start_dpg_mode(struct amdgpu_device *adev, bool indirect)
> > >   		UVD_MASTINT_EN__VCPU_EN_MASK, 0, indirect);
> > >   	if (indirect)
> > > -		psp_update_vcn_sram(adev, 0, adev->vcn.inst->dpg_sram_gpu_addr,
> > > -				    (uint32_t)((uintptr_t)adev->vcn.inst->dpg_sram_curr_addr -
> > > -					       (uintptr_t)adev->vcn.inst->dpg_sram_cpu_addr));
> > > +		amdgpu_vcn_psp_update_sram(adev, 0);
> > >   	/* force RBC into idle state */
> > >   	rb_bufsz = order_base_2(ring->ring_size);
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > > index bb1875f926f1..79d40203c952 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> > > @@ -912,9 +912,7 @@ static int vcn_v2_5_start_dpg_mode(struct amdgpu_device *adev, int inst_idx, boo
> > >   		UVD_MASTINT_EN__VCPU_EN_MASK, 0, indirect);
> > >   	if (indirect)
> > > -		psp_update_vcn_sram(adev, inst_idx, adev->vcn.inst[inst_idx].dpg_sram_gpu_addr,
> > > -				    (uint32_t)((uintptr_t)adev->vcn.inst[inst_idx].dpg_sram_curr_addr -
> > > -					       (uintptr_t)adev->vcn.inst[inst_idx].dpg_sram_cpu_addr));
> > > +		amdgpu_vcn_psp_update_sram(adev, inst_idx);
> > >   	ring = &adev->vcn.inst[inst_idx].ring_dec;
> > >   	/* force RBC into idle state */
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > > index c8f63b3c6f69..9e1570648b6c 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > > @@ -1037,9 +1037,7 @@ static int vcn_v3_0_start_dpg_mode(struct amdgpu_device *adev, int inst_idx, boo
> > >   		VCN, inst_idx, mmUVD_VCPU_CNTL), tmp, 0, indirect);
> > >   	if (indirect)
> > > -		psp_update_vcn_sram(adev, inst_idx, adev->vcn.inst[inst_idx].dpg_sram_gpu_addr,
> > > -			(uint32_t)((uintptr_t)adev->vcn.inst[inst_idx].dpg_sram_curr_addr -
> > > -				(uintptr_t)adev->vcn.inst[inst_idx].dpg_sram_cpu_addr));
> > > +		amdgpu_vcn_psp_update_sram(adev, inst_idx);
> > >   	ring = &adev->vcn.inst[inst_idx].ring_dec;
> > >   	/* force RBC into idle state */
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> > > index 2db73a964031..4febc7629512 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> > > @@ -993,9 +993,7 @@ static int vcn_v4_0_start_dpg_mode(struct amdgpu_device *adev, int inst_idx, boo
> > >   	if (indirect)
> > > -		psp_update_vcn_sram(adev, inst_idx, adev->vcn.inst[inst_idx].dpg_sram_gpu_addr,
> > > -			(uint32_t)((uintptr_t)adev->vcn.inst[inst_idx].dpg_sram_curr_addr -
> > > -				(uintptr_t)adev->vcn.inst[inst_idx].dpg_sram_cpu_addr));
> > > +		amdgpu_vcn_psp_update_sram(adev, inst_idx);
> > >   	ring = &adev->vcn.inst[inst_idx].ring_enc[0];
> > > 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 5d67b8b8a3d6..ce8c766dcc73 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
> > > @@ -778,9 +778,7 @@ static int vcn_v4_0_3_start_dpg_mode(struct amdgpu_device *adev, int inst_idx, b
> > >   		UVD_MASTINT_EN__VCPU_EN_MASK, 0, indirect);
> > >   	if (indirect)
> > > -		psp_update_vcn_sram(adev, 0, adev->vcn.inst[inst_idx].dpg_sram_gpu_addr,
> > > -			(uint32_t)((uintptr_t)adev->vcn.inst[inst_idx].dpg_sram_curr_addr -
> > > -				(uintptr_t)adev->vcn.inst[inst_idx].dpg_sram_cpu_addr));
> > > +		amdgpu_vcn_psp_update_sram(adev, inst_idx);
> 
> This patch breaks this usecase. Here it has uses a specific UCODE ID (here
> it's always VCN0_RAM), but the buffers are different. In general, instance
> ids cannot be associated with the UCODE ID to be used.

Thanks for pointing this out.

Regards,
Lang

> Thanks,
> Lijo
> 
> > >   	ring = &adev->vcn.inst[inst_idx].ring_enc[0];
> > > -- 
> > > 2.25.1
> > > 

  reply	other threads:[~2023-07-07 11:12 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-27  4:48 [PATCH 1/2] drm/amdgpu: extract a PSP function to execute IP FW loading commands Lang Yu
2023-06-27  4:48 ` [PATCH 2/2] drm/amdgpu: use psp_execute_load_ip_fw_cmd_buf instead Lang Yu
2023-06-29  8:14   ` Lang Yu
2023-07-07 10:42     ` Lazar, Lijo
2023-07-07 11:11       ` Lang Yu [this message]
2023-07-05 20:10   ` Leo Liu
2023-06-29  8:14 ` [PATCH 1/2] drm/amdgpu: extract a PSP function to execute IP FW loading commands Lang Yu
2023-07-05 16:05 ` Gopalakrishnan, Veerabadhran (Veera)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZKfy+kffRgVnsXZR@lang-desktop \
    --to=lang.yu@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=lijo.lazar@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.