[PATCH] drm/amdgpu/acpi: unify ATCS handling

Lijo Lazar lijo.lazar at amd.com
Thu May 20 08:15:55 UTC 2021



On 5/20/2021 2:07 AM, Alex Deucher wrote:
> Treat it like ATIF and check both the dGPU and APU for
> the method.  This is required because ATCS may be hung
> off of the APU in ACPI on A+A systems.
> 
> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h      |  17 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 126 ++++++++++++++++-------
>   2 files changed, 92 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 3147c1c935c8..b92eb068be12 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -269,6 +269,7 @@ struct amdgpu_irq_src;
>   struct amdgpu_fpriv;
>   struct amdgpu_bo_va_mapping;
>   struct amdgpu_atif;
> +struct amdgpu_atcs;
>   struct kfd_vm_fault_info;
>   struct amdgpu_hive_info;
>   struct amdgpu_reset_context;
> @@ -685,20 +686,6 @@ struct amdgpu_vram_scratch {
>   	u64				gpu_addr;
>   };
>   
> -/*
> - * ACPI
> - */
> -struct amdgpu_atcs_functions {
> -	bool get_ext_state;
> -	bool pcie_perf_req;
> -	bool pcie_dev_rdy;
> -	bool pcie_bus_width;
> -};
> -
> -struct amdgpu_atcs {
> -	struct amdgpu_atcs_functions functions;
> -};
> -
>   /*
>    * CGS
>    */
> @@ -829,7 +816,7 @@ struct amdgpu_device {
>   	struct amdgpu_i2c_chan		*i2c_bus[AMDGPU_MAX_I2C_BUS];
>   	struct debugfs_blob_wrapper     debugfs_vbios_blob;
>   	struct amdgpu_atif		*atif;
> -	struct amdgpu_atcs		atcs;
> +	struct amdgpu_atcs		*atcs;
>   	struct mutex			srbm_mutex;
>   	/* GRBM index mutex. Protects concurrent access to GRBM index */
>   	struct mutex                    grbm_idx_mutex;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> index bf2939b6eb43..cc8bf2ac77d7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
> @@ -71,12 +71,25 @@ struct amdgpu_atif {
>   	struct amdgpu_dm_backlight_caps backlight_caps;
>   };
>   
> +struct amdgpu_atcs_functions {
> +	bool get_ext_state;
> +	bool pcie_perf_req;
> +	bool pcie_dev_rdy;
> +	bool pcie_bus_width;
> +};
> +
> +struct amdgpu_atcs {
> +	acpi_handle handle;
> +
> +	struct amdgpu_atcs_functions functions;
> +};
> +
>   /* Call the ATIF method
>    */
>   /**
>    * amdgpu_atif_call - call an ATIF method
>    *
> - * @atif: acpi handle
> + * @atif: atif structure
>    * @function: the ATIF function to execute
>    * @params: ATIF function params
>    *
> @@ -236,6 +249,35 @@ static acpi_handle amdgpu_atif_probe_handle(acpi_handle dhandle)
>   	return handle;
>   }
>   
> +static acpi_handle amdgpu_atcs_probe_handle(acpi_handle dhandle)
> +{
> +	acpi_handle handle = NULL;
> +	char acpi_method_name[255] = { 0 };
> +	struct acpi_buffer buffer = { sizeof(acpi_method_name), acpi_method_name };
> +	acpi_status status;
> +
> +	/* For PX/HG systems, ATCS and ATPX are in the iGPU's namespace, on dGPU only
> +	 * systems, ATIF is in the dGPU's namespace.
> +	 */
> +	status = acpi_get_handle(dhandle, "ATCS", &handle);
> +	if (ACPI_SUCCESS(status))
> +		goto out;
> +
> +	if (amdgpu_has_atpx()) {
> +		status = acpi_get_handle(amdgpu_atpx_get_dhandle(), "ATCS",
> +					 &handle);
> +		if (ACPI_SUCCESS(status))
> +			goto out;
> +	}

Going per the comment, it's better to reorder. If atpx(), check in iGPU 
space first, otherwise go to device's namespace.

-- 
Thanks,
Lijo

> +	DRM_DEBUG_DRIVER("No ATCS handle found\n");
> +	return NULL;
> +out:
> +	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
> +	DRM_DEBUG_DRIVER("Found ATCS handle %s\n", acpi_method_name);
> +	return handle;
> +}
> +
>   /**
>    * amdgpu_atif_get_notification_params - determine notify configuration
>    *
> @@ -485,14 +527,15 @@ static int amdgpu_atif_handler(struct amdgpu_device *adev,
>   /**
>    * amdgpu_atcs_call - call an ATCS method
>    *
> - * @handle: acpi handle
> + * @atcs: atcs structure
>    * @function: the ATCS function to execute
>    * @params: ATCS function params
>    *
>    * Executes the requested ATCS function (all asics).
>    * Returns a pointer to the acpi output buffer.
>    */
> -static union acpi_object *amdgpu_atcs_call(acpi_handle handle, int function,
> +static union acpi_object *amdgpu_atcs_call(struct amdgpu_atcs *atcs,
> +					   int function,
>   					   struct acpi_buffer *params)
>   {
>   	acpi_status status;
> @@ -516,7 +559,7 @@ static union acpi_object *amdgpu_atcs_call(acpi_handle handle, int function,
>   		atcs_arg_elements[1].integer.value = 0;
>   	}
>   
> -	status = acpi_evaluate_object(handle, "ATCS", &atcs_arg, &buffer);
> +	status = acpi_evaluate_object(atcs->handle, "ATCS", &atcs_arg, &buffer);
>   
>   	/* Fail only if calling the method fails and ATIF is supported */
>   	if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> @@ -550,7 +593,6 @@ static void amdgpu_atcs_parse_functions(struct amdgpu_atcs_functions *f, u32 mas
>   /**
>    * amdgpu_atcs_verify_interface - verify ATCS
>    *
> - * @handle: acpi handle
>    * @atcs: amdgpu atcs struct
>    *
>    * Execute the ATCS_FUNCTION_VERIFY_INTERFACE ATCS function
> @@ -558,15 +600,14 @@ static void amdgpu_atcs_parse_functions(struct amdgpu_atcs_functions *f, u32 mas
>    * (all asics).
>    * returns 0 on success, error on failure.
>    */
> -static int amdgpu_atcs_verify_interface(acpi_handle handle,
> -					struct amdgpu_atcs *atcs)
> +static int amdgpu_atcs_verify_interface(struct amdgpu_atcs *atcs)
>   {
>   	union acpi_object *info;
>   	struct atcs_verify_interface output;
>   	size_t size;
>   	int err = 0;
>   
> -	info = amdgpu_atcs_call(handle, ATCS_FUNCTION_VERIFY_INTERFACE, NULL);
> +	info = amdgpu_atcs_call(atcs, ATCS_FUNCTION_VERIFY_INTERFACE, NULL);
>   	if (!info)
>   		return -EIO;
>   
> @@ -603,8 +644,10 @@ static int amdgpu_atcs_verify_interface(acpi_handle handle,
>    */
>   bool amdgpu_acpi_is_pcie_performance_request_supported(struct amdgpu_device *adev)
>   {
> -	struct amdgpu_atcs *atcs = &adev->atcs;
> +	struct amdgpu_atcs *atcs = adev->atcs;
>   
> +	if (!atcs)
> +		return false;
>   	if (atcs->functions.pcie_perf_req && atcs->functions.pcie_dev_rdy)
>   		return true;
>   
> @@ -622,19 +665,15 @@ bool amdgpu_acpi_is_pcie_performance_request_supported(struct amdgpu_device *ade
>    */
>   int amdgpu_acpi_pcie_notify_device_ready(struct amdgpu_device *adev)
>   {
> -	acpi_handle handle;
>   	union acpi_object *info;
> -	struct amdgpu_atcs *atcs = &adev->atcs;
> +	struct amdgpu_atcs *atcs = adev->atcs;
>   
> -	/* Get the device handle */
> -	handle = ACPI_HANDLE(&adev->pdev->dev);
> -	if (!handle)
> +	if (!atcs)
>   		return -EINVAL;
> -
>   	if (!atcs->functions.pcie_dev_rdy)
>   		return -EINVAL;
>   
> -	info = amdgpu_atcs_call(handle, ATCS_FUNCTION_PCIE_DEVICE_READY_NOTIFICATION, NULL);
> +	info = amdgpu_atcs_call(atcs, ATCS_FUNCTION_PCIE_DEVICE_READY_NOTIFICATION, NULL);
>   	if (!info)
>   		return -EIO;
>   
> @@ -657,21 +696,18 @@ int amdgpu_acpi_pcie_notify_device_ready(struct amdgpu_device *adev)
>   int amdgpu_acpi_pcie_performance_request(struct amdgpu_device *adev,
>   					 u8 perf_req, bool advertise)
>   {
> -	acpi_handle handle;
>   	union acpi_object *info;
> -	struct amdgpu_atcs *atcs = &adev->atcs;
> +	struct amdgpu_atcs *atcs = adev->atcs;
>   	struct atcs_pref_req_input atcs_input;
>   	struct atcs_pref_req_output atcs_output;
>   	struct acpi_buffer params;
>   	size_t size;
>   	u32 retry = 3;
>   
> -	if (amdgpu_acpi_pcie_notify_device_ready(adev))
> +	if (!atcs)
>   		return -EINVAL;
>   
> -	/* Get the device handle */
> -	handle = ACPI_HANDLE(&adev->pdev->dev);
> -	if (!handle)
> +	if (amdgpu_acpi_pcie_notify_device_ready(adev))
>   		return -EINVAL;
>   
>   	if (!atcs->functions.pcie_perf_req)
> @@ -691,7 +727,7 @@ int amdgpu_acpi_pcie_performance_request(struct amdgpu_device *adev,
>   	params.pointer = &atcs_input;
>   
>   	while (retry--) {
> -		info = amdgpu_atcs_call(handle, ATCS_FUNCTION_PCIE_PERFORMANCE_REQUEST, &params);
> +		info = amdgpu_atcs_call(atcs, ATCS_FUNCTION_PCIE_PERFORMANCE_REQUEST, &params);
>   		if (!info)
>   			return -EIO;
>   
> @@ -767,32 +803,26 @@ static int amdgpu_acpi_event(struct notifier_block *nb,
>    */
>   int amdgpu_acpi_init(struct amdgpu_device *adev)
>   {
> -	acpi_handle handle, atif_handle;
> +	acpi_handle handle, atif_handle, atcs_handle;
>   	struct amdgpu_atif *atif;
> -	struct amdgpu_atcs *atcs = &adev->atcs;
> +	struct amdgpu_atcs *atcs;
>   	int ret;
>   
>   	/* Get the device handle */
>   	handle = ACPI_HANDLE(&adev->pdev->dev);
>   
> -	if (!adev->bios || !handle)
> +	if (!adev->bios)
>   		return 0;
>   
> -	/* Call the ATCS method */
> -	ret = amdgpu_atcs_verify_interface(handle, atcs);
> -	if (ret) {
> -		DRM_DEBUG_DRIVER("Call to ATCS verify_interface failed: %d\n", ret);
> -	}
> -
>   	/* Probe for ATIF, and initialize it if found */
>   	atif_handle = amdgpu_atif_probe_handle(handle);
>   	if (!atif_handle)
> -		goto out;
> +		goto atcs;
>   
>   	atif = kzalloc(sizeof(*atif), GFP_KERNEL);
>   	if (!atif) {
>   		DRM_WARN("Not enough memory to initialize ATIF\n");
> -		goto out;
> +		goto atcs;
>   	}
>   	atif->handle = atif_handle;
>   
> @@ -801,7 +831,7 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
>   	if (ret) {
>   		DRM_DEBUG_DRIVER("Call to ATIF verify_interface failed: %d\n", ret);
>   		kfree(atif);
> -		goto out;
> +		goto atcs;
>   	}
>   	adev->atif = atif;
>   
> @@ -810,7 +840,8 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
>   		if (amdgpu_device_has_dc_support(adev)) {
>   #if defined(CONFIG_DRM_AMD_DC)
>   			struct amdgpu_display_manager *dm = &adev->dm;
> -			atif->bd = dm->backlight_dev;
> +			if (dm->backlight_dev)
> +				atif->bd = dm->backlight_dev;
>   #endif
>   		} else {
>   			struct drm_encoder *tmp;
> @@ -862,6 +893,28 @@ int amdgpu_acpi_init(struct amdgpu_device *adev)
>   		atif->backlight_caps.caps_valid = false;
>   	}
>   
> +atcs:
> +	/* Probe for ATCS, and initialize it if found */
> +	atcs_handle = amdgpu_atcs_probe_handle(handle);
> +	if (!atcs_handle)
> +		goto out;
> +
> +	atcs = kzalloc(sizeof(*atcs), GFP_KERNEL);
> +	if (!atif) {
> +		DRM_WARN("Not enough memory to initialize ATCS\n");
> +		goto out;
> +	}
> +	atcs->handle = atcs_handle;
> +
> +	/* Call the ATCS method */
> +	ret = amdgpu_atcs_verify_interface(atcs);
> +	if (ret) {
> +		DRM_DEBUG_DRIVER("Call to ATCS verify_interface failed: %d\n", ret);
> +		kfree(atcs);
> +		goto out;
> +	}
> +	adev->atcs = atcs;
> +
>   out:
>   	adev->acpi_nb.notifier_call = amdgpu_acpi_event;
>   	register_acpi_notifier(&adev->acpi_nb);
> @@ -892,6 +945,7 @@ void amdgpu_acpi_fini(struct amdgpu_device *adev)
>   {
>   	unregister_acpi_notifier(&adev->acpi_nb);
>   	kfree(adev->atif);
> +	kfree(adev->atcs);
>   }
>   
>   /**
> 




More information about the amd-gfx mailing list