Re: [PATCH 1/2] drm/amdgpu/vcn1.0: use its own idle handler and begin use funcs

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

Am 13.12.19 um 15:42 schrieb Leo Liu:
> Because VCN1.0 power management and DPG mode are managed together with
> JPEG1.0 under both HW and FW, so separated them from general VCN code.
> Also the multiple instances case got removed, since VCN1.0 HW just have
> a single instance.
>
> v2: override work func with vcn1.0's own
>
> Signed-off-by: Leo Liu <leo.liu@xxxxxxx>

Reviewed-by: Christian König <christian.koenig@xxxxxxx>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c |  3 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  3 +
>   drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c  |  3 +-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c   | 90 ++++++++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h   |  2 +
>   5 files changed, 95 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 428cfd58b37d..717f0a218c5d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -39,9 +39,6 @@
>   #include "vcn/vcn_1_0_offset.h"
>   #include "vcn/vcn_1_0_sh_mask.h"
>   
> -/* 1 second timeout */
> -#define VCN_IDLE_TIMEOUT	msecs_to_jiffies(1000)
> -
>   /* Firmware Names */
>   #define FIRMWARE_RAVEN		"amdgpu/raven_vcn.bin"
>   #define FIRMWARE_PICASSO	"amdgpu/picasso_vcn.bin"
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 402a5046b985..3484ead62046 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -56,6 +56,9 @@
>   #define VCN_VID_IP_ADDRESS_2_0		0x0
>   #define VCN_AON_IP_ADDRESS_2_0		0x30000
>   
> +/* 1 second timeout */
> +#define VCN_IDLE_TIMEOUT	msecs_to_jiffies(1000)
> +
>   #define RREG32_SOC15_DPG_MODE(ip, inst, reg, mask, sram_sel) 				\
>   	({	WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_MASK, mask); 			\
>   		WREG32_SOC15(ip, inst, mmUVD_DPG_LMA_CTL, 				\
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c b/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
> index a141408dfb23..0debfd9f428c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v1_0.c
> @@ -25,6 +25,7 @@
>   #include "amdgpu_jpeg.h"
>   #include "soc15.h"
>   #include "soc15d.h"
> +#include "vcn_v1_0.h"
>   
>   #include "vcn/vcn_1_0_offset.h"
>   #include "vcn/vcn_1_0_sh_mask.h"
> @@ -561,7 +562,7 @@ static const struct amdgpu_ring_funcs jpeg_v1_0_decode_ring_vm_funcs = {
>   	.insert_start = jpeg_v1_0_decode_ring_insert_start,
>   	.insert_end = jpeg_v1_0_decode_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
> -	.begin_use = amdgpu_vcn_ring_begin_use,
> +	.begin_use = vcn_v1_0_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
>   	.emit_wreg = jpeg_v1_0_decode_ring_emit_wreg,
>   	.emit_reg_wait = jpeg_v1_0_decode_ring_emit_reg_wait,
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index 652cecc030b3..3b025a3f8c7d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -25,6 +25,7 @@
>   
>   #include "amdgpu.h"
>   #include "amdgpu_vcn.h"
> +#include "amdgpu_pm.h"
>   #include "soc15.h"
>   #include "soc15d.h"
>   #include "soc15_common.h"
> @@ -51,6 +52,8 @@ static int vcn_v1_0_set_powergating_state(void *handle, enum amd_powergating_sta
>   static int vcn_v1_0_pause_dpg_mode(struct amdgpu_device *adev,
>   				struct dpg_pause_state *new_state);
>   
> +static void vcn_v1_0_idle_work_handler(struct work_struct *work);
> +
>   /**
>    * vcn_v1_0_early_init - set function pointers
>    *
> @@ -105,6 +108,9 @@ static int vcn_v1_0_sw_init(void *handle)
>   	if (r)
>   		return r;
>   
> +	/* Override the work func */
> +	adev->vcn.idle_work.work.func = vcn_v1_0_idle_work_handler;
> +
>   	if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) {
>   		const struct common_firmware_header *hdr;
>   		hdr = (const struct common_firmware_header *)adev->vcn.fw->data;
> @@ -1758,6 +1764,86 @@ static int vcn_v1_0_set_powergating_state(void *handle,
>   	return ret;
>   }
>   
> +static void vcn_v1_0_idle_work_handler(struct work_struct *work)
> +{
> +	struct amdgpu_device *adev =
> +		container_of(work, struct amdgpu_device, vcn.idle_work.work);
> +	unsigned int fences = 0, i;
> +
> +	for (i = 0; i < adev->vcn.num_enc_rings; ++i)
> +		fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_enc[i]);
> +
> +	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> +		struct dpg_pause_state new_state;
> +
> +		if (fences)
> +			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> +
> +		if (amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec))
> +			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +
> +		adev->vcn.pause_dpg_mode(adev, &new_state);
> +	}
> +
> +	fences += amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec);
> +	fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_dec);
> +
> +	if (fences == 0) {
> +		amdgpu_gfx_off_ctrl(adev, true);
> +		if (adev->pm.dpm_enabled)
> +			amdgpu_dpm_enable_uvd(adev, false);
> +		else
> +			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> +			       AMD_PG_STATE_GATE);
> +	} else {
> +		schedule_delayed_work(&adev->vcn.idle_work, VCN_IDLE_TIMEOUT);
> +	}
> +}
> +
> +void vcn_v1_0_ring_begin_use(struct amdgpu_ring *ring)
> +{
> +	struct amdgpu_device *adev = ring->adev;
> +	bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);
> +
> +	if (set_clocks) {
> +		amdgpu_gfx_off_ctrl(adev, false);
> +		if (adev->pm.dpm_enabled)
> +			amdgpu_dpm_enable_uvd(adev, true);
> +		else
> +			amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
> +			       AMD_PG_STATE_UNGATE);
> +	}
> +
> +	if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> +		struct dpg_pause_state new_state;
> +		unsigned int fences = 0, i;
> +
> +		for (i = 0; i < adev->vcn.num_enc_rings; ++i)
> +			fences += amdgpu_fence_count_emitted(&adev->vcn.inst->ring_enc[i]);
> +
> +		if (fences)
> +			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.fw_based = VCN_DPG_STATE__UNPAUSE;
> +
> +		if (amdgpu_fence_count_emitted(&adev->jpeg.inst->ring_dec))
> +			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +		else
> +			new_state.jpeg = VCN_DPG_STATE__UNPAUSE;
> +
> +		if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
> +			new_state.fw_based = VCN_DPG_STATE__PAUSE;
> +		else if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_JPEG)
> +			new_state.jpeg = VCN_DPG_STATE__PAUSE;
> +
> +		adev->vcn.pause_dpg_mode(adev, &new_state);
> +	}
> +}
> +
>   static const struct amd_ip_funcs vcn_v1_0_ip_funcs = {
>   	.name = "vcn_v1_0",
>   	.early_init = vcn_v1_0_early_init,
> @@ -1804,7 +1890,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_dec_ring_vm_funcs = {
>   	.insert_start = vcn_v1_0_dec_ring_insert_start,
>   	.insert_end = vcn_v1_0_dec_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
> -	.begin_use = amdgpu_vcn_ring_begin_use,
> +	.begin_use = vcn_v1_0_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
>   	.emit_wreg = vcn_v1_0_dec_ring_emit_wreg,
>   	.emit_reg_wait = vcn_v1_0_dec_ring_emit_reg_wait,
> @@ -1836,7 +1922,7 @@ static const struct amdgpu_ring_funcs vcn_v1_0_enc_ring_vm_funcs = {
>   	.insert_nop = amdgpu_ring_insert_nop,
>   	.insert_end = vcn_v1_0_enc_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
> -	.begin_use = amdgpu_vcn_ring_begin_use,
> +	.begin_use = vcn_v1_0_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
>   	.emit_wreg = vcn_v1_0_enc_ring_emit_wreg,
>   	.emit_reg_wait = vcn_v1_0_enc_ring_emit_reg_wait,
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
> index 2a497a7a4840..f67d7391fc21 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.h
> @@ -24,6 +24,8 @@
>   #ifndef __VCN_V1_0_H__
>   #define __VCN_V1_0_H__
>   
> +void vcn_v1_0_ring_begin_use(struct amdgpu_ring *ring);
> +
>   extern const struct amdgpu_ip_block_version vcn_v1_0_ip_block;
>   
>   #endif

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx