Re: [PATCH 14/15] platform/x86/amd/pmf: Add PMF-AMDSFH interface for HPD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/22/2023 14:04, Mario Limonciello wrote:
On 9/22/2023 12:50, Shyam Sundar S K wrote:
From: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>

AMDSFH has information about the User presence information via the Human
Presence Detection (HPD) sensor which is part of the AMD sensor fusion hub.
Add PMF and AMDSFH interface to get this information.

Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>
---
  drivers/hid/amd-sfh-hid/amd_sfh_common.h      |  5 ++++
  drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c |  2 +-
  drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 11 ++++++++
  .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 28 +++++++++++++++++++
  .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    |  1 +
  drivers/platform/x86/amd/pmf/spc.c            | 21 ++++++++++++++
  include/linux/amd-pmf-io.h                    | 22 ++++++++++++++-
  7 files changed, 88 insertions(+), 2 deletions(-)

Somthing else I noticed about this change.

I believe you should be adding to Kconfig:
depends on AMD_SFH_HID


diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index 2643bb14fee2..cd57037bf217 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -37,6 +37,10 @@ struct amd_mp2_sensor_info {
      dma_addr_t dma_address;
  };
+struct sfh_dev_status {
+    bool is_hpd_present;
+};
+
  struct amd_mp2_dev {
      struct pci_dev *pdev;
      struct amdtp_cl_data *cl_data;
@@ -47,6 +51,7 @@ struct amd_mp2_dev {
      struct amd_input_data in_data;
      /* mp2 active control status */
      u32 mp2_acs;
+    struct sfh_dev_status dev_en;
  };
  struct amd_mp2_ops {
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
index 06bdcf072d10..d7467c41ad3b 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
@@ -132,7 +132,7 @@ static void get_common_inputs(struct common_input_property *common, int report_i
      common->event_type = HID_USAGE_SENSOR_EVENT_DATA_UPDATED_ENUM;
  }
-static int float_to_int(u32 flt32_val)
+int float_to_int(u32 flt32_val)
  {
      int fraction, shift, mantissa, sign, exp, zeropre;
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
index e9c6413af24a..9c623456ee12 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
@@ -73,6 +73,12 @@ static int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
      int i, status;
      for (i = 0; i < cl_data->num_hid_devices; i++) {
+        switch (cl_data->sensor_idx[i]) {
+        case HPD_IDX:
+            privdata->dev_en.is_hpd_present = false;
+            break;
+        }
+
          if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
              privdata->mp2_ops->stop(privdata, cl_data->sensor_idx[i]);
              status = amd_sfh_wait_for_response
@@ -178,6 +184,11 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
              rc = amdtp_hid_probe(i, cl_data);
              if (rc)
                  goto cleanup;
+            switch (cl_data->sensor_idx[i]) {
+            case HPD_IDX:
+            privdata->dev_en.is_hpd_present = true;

Looks like this is missing a tab.

+                break;
+            }
          }
          dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
              cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]), diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 4f81ef2d4f56..63a5bbca5a09 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -7,11 +7,14 @@
   *
   * Author: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>
   */
+#include <linux/amd-pmf-io.h>
  #include <linux/io-64-nonatomic-lo-hi.h>
  #include <linux/iopoll.h>
  #include "amd_sfh_interface.h"
+static struct amd_mp2_dev *emp2;
+
  static int amd_sfh_wait_response(struct amd_mp2_dev *mp2, u8 sid, u32 cmd_id)
  {
      struct sfh_cmd_response cmd_resp;
@@ -76,4 +79,29 @@ static struct amd_mp2_ops amd_sfh_ops = {
  void sfh_interface_init(struct amd_mp2_dev *mp2)
  {
      mp2->mp2_ops = &amd_sfh_ops;
+    emp2 = mp2;
+}
+
+static int amd_sfh_hpd_info(u8 *user_present)
+{
+    if (emp2 && emp2->dev_en.is_hpd_present) {
+        struct hpd_status hpdstatus;
+
+        hpdstatus.val = readl(emp2->mmio + AMD_C2P_MSG(4));
+        *user_present = hpdstatus.shpd.presence;
+        return 0;
+    }
+    return  -ENODEV;

Extra whitespace here.

+}
+

IMO you should add some kernel doc here or in the header as good practice for exported symbols.

+int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
+{
+    if (sfh_info) {
+        switch (op) {
+        case MT_HPD:
+            return amd_sfh_hpd_info(&sfh_info->user_present);
+        }
+    }
+    return -1;

This looks like it should probably be -EINVAL.

  }
+EXPORT_SYMBOL_GPL(amd_get_sfh_info);
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 9d31d5b510eb..8a36386e6bce 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -149,6 +149,7 @@ struct hpd_status {
      };
  };
+int float_to_int(u32 flt32_val);
  void sfh_interface_init(struct amd_mp2_dev *mp2);
  void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
  #endif
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index 5f3ab1ce09d2..97293ae25cf5 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -48,6 +48,7 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *       dev_dbg(dev->dev, "Primary Display State : %s\n", in->ev_info.display_state ?
              "Connected" : "disconnected/unknown");
      dev_dbg(dev->dev, "LID State : %s\n", in->ev_info.lid_state ? "Close" : "Open"); +    dev_dbg(dev->dev, "User Presence : %s\n", in->ev_info.user_present ? "Present" : "Away");
      dev_dbg(dev->dev, "==== TA inputs END ====\n");
  }
  #else
@@ -156,6 +157,25 @@ static void amd_pmf_get_gpu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_ta
      in->ev_info.display_state = dev->gfx_data.con_status[0];
  }
+static void amd_pmf_get_sensor_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
+{
+    struct amd_sfh_info sfh_info;
+
+    /* get HPD data */
+    amd_get_sfh_info(&sfh_info, MT_HPD);

amd_get_sfh_info() is int.

Shouldn't you look for the return code here?

+    switch (sfh_info.user_present) {
+    case SFH_NOT_DETECTED:
+        in->ev_info.user_present = 0xff; /* assume no sensors connected */
+        break;
+    case SFH_USER_PRESENT:
+        in->ev_info.user_present = 1;
+        break;
+    case SFH_USER_AWAY:
+        in->ev_info.user_present = 0;
+        break;
+    }
+}
+
  void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in)
  {
      /* TA side lid open is 1 and close is 0, hence the ! here */
@@ -165,4 +185,5 @@ void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_tab
      amd_pmf_get_battery_info(dev, in);
      amd_pmf_get_slider_info(dev, in);
      amd_pmf_get_gpu_info(dev, in);
+    amd_pmf_get_sensor_info(dev, in);
  }
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
index ecae387ddaa6..4f82973f6ad2 100644
--- a/include/linux/amd-pmf-io.h
+++ b/include/linux/amd-pmf-io.h
@@ -5,7 +5,8 @@
   * Copyright (c) 2023, Advanced Micro Devices, Inc.
   * All Rights Reserved.
   *
- * Author: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
+ * Authors: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
+ *          Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>
   */
  #ifndef AMD_PMF_IO_H
@@ -26,4 +27,23 @@ struct amd_gpu_pmf_data {
  int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf);
  int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf);
+
+/* amd-sfh */
+enum sfh_message_type {
+    MT_HPD,
+};
+
+enum hpd_info {
+    SFH_NOT_DETECTED,
+    SFH_USER_PRESENT,
+    SFH_USER_AWAY
+};
+
+struct amd_sfh_info {
+    u8 user_present;
+    /* add future caps below */
+};
+
+int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op);
+
  #endif





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux