Re: [PATCH 13/15] platform/x86/amd/pmf: Add PMF-AMDGPU set interface

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

 



Am 26.09.23 um 14:56 schrieb Hans de Goede:
Hi,

On 9/26/23 13:24, Shyam Sundar S K wrote:
Hi Hans,

On 9/26/2023 4:05 PM, Hans de Goede wrote:
Hi,

On 9/22/23 19:50, Shyam Sundar S K wrote:
For the Smart PC Solution to fully work, it has to enact to the actions
coming from TA. Add the initial code path for set interface to AMDGPU.

Co-developed-by: Mario Limonciello <mario.limonciello@xxxxxxx>
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 21 +++++++++++++++++++++
  drivers/platform/x86/amd/pmf/pmf.h      |  2 ++
  drivers/platform/x86/amd/pmf/tee-if.c   | 19 +++++++++++++++++--
  include/linux/amd-pmf-io.h              |  1 +
  4 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
index 232d11833ddc..5c567bff0548 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
@@ -68,3 +68,24 @@ int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
  	return 0;
  }
  EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
+
+int amd_pmf_set_gfx_data(struct amd_gpu_pmf_data *pmf)
+{
+	struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
+	struct amdgpu_device *adev = drm_to_adev(drm_dev);
+	struct backlight_device *bd;
+
+	if (!(adev->flags & AMD_IS_APU)) {
+		DRM_ERROR("PMF-AMDGPU interface not supported\n");
+		return -ENODEV;
+	}
+
+	bd = backlight_device_get_by_type(BACKLIGHT_RAW);
+	if (!bd)
+		return -ENODEV;
This assumes that the backlight is always controller by the amdgpu's
native backlight driver, but it might e.g. also be handled by
eacpi-video or by nvidia_wmi_ec_backlight (when using an AMD APU +
nvidia dgpu).
PMF is meant for AMD APUs(atleast for now) and the _HID will only be
made visible if its AMD laptop. So using amdgpu's native BACKLIGHT_RAW
should be safe, right?
Users can pass say acpi_backlight=video and use the acpi_video
driver for backlight control instead of the native GPU backlight
control.

For now what should be done here is to call acpi_video_get_backlight_type()
and then translate the return value from this into a backlight-type:

         acpi_backlight_video		-> BACKLIGHT_FIRMWARE
         acpi_backlight_vendor,		-> BACKLIGHT_PLATFORM
         acpi_backlight_native,		-> BACKLIGHT_RAW
         acpi_backlight_nvidia_wmi_ec,	-> BACKLIGHT_FIRMWARE
         acpi_backlight_apple_gmux,	-> BACKLIGHT_PLATFORM

I can add this change in the v2, do you insist on this?
Insist is a strong word, but I think that it is a good idea to have
this. Evenutally it looks like this code will need to either integrate with
the drm drivers lot more; or the drm core needs to export some special
hooks for this which the PMF code can then call.

Actually thinking more about this, I think that the right thing to do
here is make some code register brightness control as a cooling device
(which I think is already done in some cases) and then have the PMF
code use the cooling-device APIs for this.

IMHO that would be a much cleaner solution then this hack.

Yeah, fully agree with Hans. This looks like a rather extreme hack to me.

Apart from that what exactly is this thing supposed to do? Prevent overheating by reducing the brightness?

Regards,
Christian.


Regards,

Hans



Thanks,
Shyam

Also I'm worried about probe order here, this code currently assumes
that the GPU or other backlight driver has loaded before this runs,
which is not necessarily the case.

I think that if the backlight_device_get_by_type() fails this
should be retried say every 10 seconds from some delayed workqueue
for at least a couple of minutes after boot.

Regards,

Hans




+
+	backlight_device_set_brightness(bd, pmf->brightness);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(amd_pmf_set_gfx_data);
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 9032df4ba48a..ce89cc0daa5a 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -73,6 +73,7 @@
  #define PMF_POLICY_STT_SKINTEMP_APU				7
  #define PMF_POLICY_STT_SKINTEMP_HS2				8
  #define PMF_POLICY_SYSTEM_STATE					9
+#define PMF_POLICY_DISPLAY_BRIGHTNESS				12
  #define PMF_POLICY_P3T						38
/* TA macros */
@@ -480,6 +481,7 @@ enum ta_pmf_error_type {
  };
struct pmf_action_table {
+	unsigned long display_brightness;
  	enum system_state system_state;
  	unsigned long spl; /* in mW */
  	unsigned long sppt; /* in mW */
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 1608996654e8..eefffff83a4c 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -79,10 +79,10 @@ static int amd_pmf_update_uevents(struct amd_pmf_dev *dev, u16 event)
  	return 0;
  }
-static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
+static int amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_result *out)
  {
  	u32 val, event = 0;
-	int idx;
+	int idx, ret;
for (idx = 0; idx < out->actions_count; idx++) {
  		val = out->actions_list[idx].value;
@@ -160,8 +160,23 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
  				dev->prev_data->system_state = 0;
  			}
  			break;
+
+		case PMF_POLICY_DISPLAY_BRIGHTNESS:
+			ret = amd_pmf_get_gfx_data(&dev->gfx_data);
+			if (ret)
+				return ret;
+
+			dev->prev_data->display_brightness = dev->gfx_data.brightness;
+			if (dev->prev_data->display_brightness != val) {
+				dev->gfx_data.brightness = val;
+				amd_pmf_set_gfx_data(&dev->gfx_data);
+				dev_dbg(dev->dev, "update DISPLAY_BRIGHTNESS : %d\n", val);
+			}
+			break;
  		}
  	}
+
+	return 0;
  }
static int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev)
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
index a2d4af231362..ecae387ddaa6 100644
--- a/include/linux/amd-pmf-io.h
+++ b/include/linux/amd-pmf-io.h
@@ -25,4 +25,5 @@ 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);
  #endif




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux