[PATCH 1/2] drm/amdgpu/acpi: unify ATCS handling (v3)
Lijo Lazar
lijo.lazar at amd.com
Fri May 21 14:22:36 UTC 2021
Thanks for clarifying!
Reviewed-by: Lijo Lazar <lijo.lazar at amd.com>
On 5/21/2021 7:17 PM, Alex Deucher wrote:
> On Fri, May 21, 2021 at 3:12 AM Lijo Lazar <lijo.lazar at amd.com> wrote:
>>
>>
>>
>> On 5/20/2021 9:26 PM, 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.
>>>
>>> v2: add back accidently removed ACPI handle check.
>>> v3: Fix incorrect atif check (Colin)
>>> Fix uninitialized variable (Colin)
>>>
>>> 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 | 128 ++++++++++++++++-------
>>> 2 files changed, 93 insertions(+), 52 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index b6435479cac8..ece1aee5a667 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -268,6 +268,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;
>>> @@ -681,20 +682,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
>>> */
>>> @@ -825,7 +812,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 6cf6231057fc..29708b5685ad 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
>>> @@ -72,12 +72,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
>>> *
>>> @@ -237,6 +250,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;
>>> + }
>>> +
>>> + 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
>>> *
>>> @@ -486,14 +528,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;
>>> @@ -517,7 +560,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) {
>>> @@ -551,7 +594,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
>>> @@ -559,15 +601,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;
>>>
>>> @@ -604,8 +645,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;
>>>
>>> @@ -623,19 +666,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;
>>>
>>> @@ -658,21 +697,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)
>>> @@ -692,7 +728,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, ¶ms);
>>> + info = amdgpu_atcs_call(atcs, ATCS_FUNCTION_PCIE_PERFORMANCE_REQUEST, ¶ms);
>>> if (!info)
>>> return -EIO;
>>>
>>> @@ -768,32 +804,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;
>>> - int ret;
>>> + struct amdgpu_atcs *atcs;
>>> + int ret = 0;
>>>
>>> /* Get the device handle */
>>> handle = ACPI_HANDLE(&adev->pdev->dev);
>>>
>>> if (!adev->bios || !handle)
>>> - 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);
>>> - }
>>> + return ret;
>>
>> Is this return ok? Is it possible not to have ACPI handle for the dGPU,
>> but has a valid handle for iGPU - like ATIF/ATCS functions that exist in
>> iGPU space?
>
> We are just checking for an ACPI handle here. Presumably if the
> device were part of the platform it would have an ACPI handle. ATIF
> and ATCS are only relevant to devices which are part of the platform.
> E.g., they wouldn't apply to a card plugged into a PCIe slot or
> thunderbolt.
>
> Alex
>
>>
>> --
>> Thanks,
>> Lijo
>>
>>> /* 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;
>>>
>>> @@ -802,7 +832,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;
>>>
>>> @@ -811,7 +841,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;
>>> @@ -863,6 +894,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 (!atcs) {
>>> + 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);
>>> @@ -893,6 +946,7 @@ void amdgpu_acpi_fini(struct amdgpu_device *adev)
>>> {
>>> unregister_acpi_notifier(&adev->acpi_nb);
>>> kfree(adev->atif);
>>> + kfree(adev->atcs);
>>> }
>>>
>>> /**
>>>
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Clijo.lazar%40amd.com%7C53d11f255bdc4a85b89508d91c5ef6c0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637572016470028947%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=Y1MV9LMAGE2dDA8c5iuRzeY3XJPAbNu%2Fka4Ot0vVD2M%3D&reserved=0
--
Thanks,
Lijo
More information about the amd-gfx
mailing list