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

2021-05-20 Thread Lijo Lazar




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 
---
  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 mutexsrbm_mutex;
/* GRBM index mutex. Protects concurrent access to GRBM index */
struct mutexgrbm_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", );
+   if (ACPI_SUCCESS(status))
+   goto out;
+
+   if (amdgpu_has_atpx()) {
+   status = acpi_get_handle(amdgpu_atpx_get_dhandle(), "ATCS",
+);
+   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, );
+   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", _arg, );

+   status = acpi_evaluate_object(atcs->handle, "ATCS", _arg, );
  
  	/* Fail only if calling the method fails and ATIF is supported */

if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
@@ 

[PATCH] drm/amdgpu/acpi: unify ATCS handling (v2)

2021-05-19 Thread Alex Deucher
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.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h  |  17 +---
 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 124 ---
 2 files changed, 91 insertions(+), 50 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 mutexsrbm_mutex;
/* GRBM index mutex. Protects concurrent access to GRBM index */
struct mutexgrbm_idx_mutex;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
index bf2939b6eb43..93f5207104ec 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", );
+   if (ACPI_SUCCESS(status))
+   goto out;
+
+   if (amdgpu_has_atpx()) {
+   status = acpi_get_handle(amdgpu_atpx_get_dhandle(), "ATCS",
+);
+   if (ACPI_SUCCESS(status))
+   goto out;
+   }
+
+   DRM_DEBUG_DRIVER("No ATCS handle found\n");
+   return NULL;
+out:
+   acpi_get_name(handle, ACPI_FULL_PATHNAME, );
+   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", _arg, );
+   status = acpi_evaluate_object(atcs->handle, "ATCS", _arg, );
 
/* 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: 

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

2021-05-19 Thread Alex Deucher
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 
---
 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 mutexsrbm_mutex;
/* GRBM index mutex. Protects concurrent access to GRBM index */
struct mutexgrbm_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", );
+   if (ACPI_SUCCESS(status))
+   goto out;
+
+   if (amdgpu_has_atpx()) {
+   status = acpi_get_handle(amdgpu_atpx_get_dhandle(), "ATCS",
+);
+   if (ACPI_SUCCESS(status))
+   goto out;
+   }
+
+   DRM_DEBUG_DRIVER("No ATCS handle found\n");
+   return NULL;
+out:
+   acpi_get_name(handle, ACPI_FULL_PATHNAME, );
+   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", _arg, );
+   status = acpi_evaluate_object(atcs->handle, "ATCS", _arg, );
 
/* 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
  *
  *