Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
Hi Hans,
Apologies for the long delay.
On 10/19/2023 12:38 AM, Hans de Goede wrote:
> Hi,
>
> I was not following this at first, so my apologies for
> jumping in in the middle of the thread:
>
>
>
>
>> +static int amd_pmf_gpu_get_cur_state(struct thermal_cooling_device
>> *cooling_dev,
>> + unsigned long *state)
>> +{
>> + struct backlight_device *bd;
>> +
>> + if (!acpi_video_backlight_use_native())
>> + return -ENODEV;
>> +
>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>> + if (!bd)
>> + return -ENODEV;
>> +
>> + *state = backlight_get_brightness(bd);
>> +
>> + return 0;
>> +}
>> +
>> +static int amd_pmf_gpu_get_max_state(struct thermal_cooling_device
>> *cooling_dev,
>> + unsigned long *state)
>> +{
>> + struct backlight_device *bd;
>> +
>> + if (!acpi_video_backlight_use_native())
>> + return -ENODEV;
>> +
>> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
>> + if (!bd)
>> + return -ENODEV;
>> +
>> + if (backlight_is_blank(bd))
>> + *state = 0;
>> + else
>> + *state = bd->props.max_brightness;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct thermal_cooling_device_ops bd_cooling_ops = {
>> + .get_max_state = amd_pmf_gpu_get_max_state,
>> + .get_cur_state = amd_pmf_gpu_get_cur_state,
>> +};
>
> So first of all, good to see that this is using the
> thermal_cooling_device APIs now, that is great thank you.
>
> But the whole idea behind using the thermal_cooling_device APIs
> is that amdgpu exports the cooling_device itself, rather then have
> the AMD PMF code export it. Now the AMD PMF code is still poking
> at the backlight_device itself, while the idea was to delegate
> this to the GPU driver.
>
> Actually seeing all the acpi_video_backlight_use_native()
> checks here, I wonder why only have this work with native backlight
> control. One step better would be to add thermal_cooling_device
> support to the backlight core in:
> drivers/video/backlight/backlight.c
>
> Then it will work with any backlight control provider!
>
>
>
> Last but not least this code MUST not call
> acpi_video_backlight_use_native()
>
> No code other then native GPU drivers must ever call
> acpi_video_backlight_use_native(). This special function
> not only checks if the native backlight control is the
> one which the detection code in drivers/acpi/video_detect.c
> has selected, it also signals to video_detect.c that
> native GPU backlight control is available.
>
> So by calling this in the AMD PMF code you are now
> telling video_detect.c that native GPU backlight control
> is available on all systems where AMD PMF runs.
>
> As I already said I really believe the whole cooling
> device should be registered somewhere else. But if you
> do end up sticking with this then you MUST replace
> the acpi_video_backlight_use_native() calls with:
>
> if (acpi_video_get_backlight_type() == acpi_backlight_native) {...}
Thank you very much for your detailed feedback. This helped.
I have moved the code from amdgpu to PMF driver which has changes for
DRM. This also has changed w.r.t thermal device change what you suggested.
I have used the checks where ever appropriate:
acpi_video_get_backlight_type() == acpi_backlight_native
Kindly take a look at v5 submission.
Thanks,
Shyam
>
> Regards,
>
> Hans
>
>
>
Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
On 10/19/2023 2:31 PM, Christian König wrote: > Am 18.10.23 um 19:05 schrieb Shyam Sundar S K: >> >> On 10/18/2023 9:37 PM, Christian König wrote: >>> Am 18.10.23 um 17:47 schrieb Mario Limonciello: On 10/18/2023 08:40, Christian König wrote: > > Am 18.10.23 um 11:28 schrieb Shyam Sundar S K: >> On 10/18/2023 2:50 PM, Ilpo Järvinen wrote: >>> On Wed, 18 Oct 2023, Shyam Sundar S K wrote: >>> In order to provide GPU inputs to TA for the Smart PC solution to work, we need to have interface between the PMF driver and the AMDGPU driver. Add the initial code path for get interface from AMDGPU. Co-developed-by: Mario Limonciello Signed-off-by: Mario Limonciello Signed-off-by: Shyam Sundar S K --- drivers/gpu/drm/amd/amdgpu/Makefile | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138 drivers/platform/x86/amd/pmf/Kconfig | 1 + drivers/platform/x86/amd/pmf/core.c | 1 + drivers/platform/x86/amd/pmf/pmf.h | 3 + drivers/platform/x86/amd/pmf/spc.c | 13 +++ drivers/platform/x86/amd/pmf/tee-if.c | 26 + include/linux/amd-pmf-io.h | 35 ++ 9 files changed, 220 insertions(+) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c create mode 100644 include/linux/amd-pmf-io.h diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile index 384b798a9bad..7fafccefbd7a 100644 --- a/drivers/gpu/drm/amd/amdgpu/Makefile +++ b/drivers/gpu/drm/amd/amdgpu/Makefile @@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o +amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o + # add asic specific block amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \ dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index a79d53bdbe13..26ffa1b4fe57 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -50,6 +50,7 @@ #include #include #include +#include #include #include diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c new file mode 100644 index ..ac981848df50 --- /dev/null +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c @@ -0,0 +1,138 @@ +/* + * Copyright 2023 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. >>> This is MIT, right? Add the required SPDX-License-Identifier line >>> for it >>> at the top of the file, thank you. >>> >> all the files in drivers/gpu/drm/amd/amdgpu/* carry the same >> license >> text. So, have retained it to maintain uniformity. > Please add the SPDX License Identifier for any file you add. >> OK. I thought to keep it same like the other files. I will change this >> to SPDX in the next revision. >> > Apart from that the whole approach of attaching this directly to > amdgpu looks extremely questionable to me. > What's the long term outlook for things that
Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
Am 18.10.23 um 19:05 schrieb Shyam Sundar S K: On 10/18/2023 9:37 PM, Christian König wrote: Am 18.10.23 um 17:47 schrieb Mario Limonciello: On 10/18/2023 08:40, Christian König wrote: Am 18.10.23 um 11:28 schrieb Shyam Sundar S K: On 10/18/2023 2:50 PM, Ilpo Järvinen wrote: On Wed, 18 Oct 2023, Shyam Sundar S K wrote: In order to provide GPU inputs to TA for the Smart PC solution to work, we need to have interface between the PMF driver and the AMDGPU driver. Add the initial code path for get interface from AMDGPU. Co-developed-by: Mario Limonciello Signed-off-by: Mario Limonciello Signed-off-by: Shyam Sundar S K --- drivers/gpu/drm/amd/amdgpu/Makefile | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138 drivers/platform/x86/amd/pmf/Kconfig | 1 + drivers/platform/x86/amd/pmf/core.c | 1 + drivers/platform/x86/amd/pmf/pmf.h | 3 + drivers/platform/x86/amd/pmf/spc.c | 13 +++ drivers/platform/x86/amd/pmf/tee-if.c | 26 + include/linux/amd-pmf-io.h | 35 ++ 9 files changed, 220 insertions(+) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c create mode 100644 include/linux/amd-pmf-io.h diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile index 384b798a9bad..7fafccefbd7a 100644 --- a/drivers/gpu/drm/amd/amdgpu/Makefile +++ b/drivers/gpu/drm/amd/amdgpu/Makefile @@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o +amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o + # add asic specific block amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \ dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index a79d53bdbe13..26ffa1b4fe57 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -50,6 +50,7 @@ #include #include #include +#include #include #include diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c new file mode 100644 index ..ac981848df50 --- /dev/null +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c @@ -0,0 +1,138 @@ +/* + * Copyright 2023 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. This is MIT, right? Add the required SPDX-License-Identifier line for it at the top of the file, thank you. all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license text. So, have retained it to maintain uniformity. Please add the SPDX License Identifier for any file you add. OK. I thought to keep it same like the other files. I will change this to SPDX in the next revision. Apart from that the whole approach of attaching this directly to amdgpu looks extremely questionable to me. What's the long term outlook for things that are needed directly from amdgpu? Is there going to be more besides the backlight and the display count/type? Yeah, that goes into the same direction as my concern. PMF is an APU only feature and will need inputs from multiple subsystems (in this case its GPU) to send changing system information to PMF TA (Trusted Application, running on ASP/PSP) as input. Its not only about the display count/type and backlight, there are many others things in pipe like the GPU engine running time, utilization percentage etc, that guide the TA to make better decisions. This series is only targeted to build the initial plubming with the subsystems inside the kernel and later keep adding changes to get more information from other subsystems. Yeah, and that's what I strongly disagree on. Don't come up with such an approach without consulting with upstream maintainers first. What you propose here is a system wide framework for improving power management, that's fine.
Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
Hi,
I was not following this at first, so my apologies for
jumping in in the middle of the thread:
> +static int amd_pmf_gpu_get_cur_state(struct thermal_cooling_device
> *cooling_dev,
> + unsigned long *state)
> +{
> + struct backlight_device *bd;
> +
> + if (!acpi_video_backlight_use_native())
> + return -ENODEV;
> +
> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
> + if (!bd)
> + return -ENODEV;
> +
> + *state = backlight_get_brightness(bd);
> +
> + return 0;
> +}
> +
> +static int amd_pmf_gpu_get_max_state(struct thermal_cooling_device
> *cooling_dev,
> + unsigned long *state)
> +{
> + struct backlight_device *bd;
> +
> + if (!acpi_video_backlight_use_native())
> + return -ENODEV;
> +
> + bd = backlight_device_get_by_type(BACKLIGHT_RAW);
> + if (!bd)
> + return -ENODEV;
> +
> + if (backlight_is_blank(bd))
> + *state = 0;
> + else
> + *state = bd->props.max_brightness;
> +
> + return 0;
> +}
> +
> +static const struct thermal_cooling_device_ops bd_cooling_ops = {
> + .get_max_state = amd_pmf_gpu_get_max_state,
> + .get_cur_state = amd_pmf_gpu_get_cur_state,
> +};
So first of all, good to see that this is using the
thermal_cooling_device APIs now, that is great thank you.
But the whole idea behind using the thermal_cooling_device APIs
is that amdgpu exports the cooling_device itself, rather then have
the AMD PMF code export it. Now the AMD PMF code is still poking
at the backlight_device itself, while the idea was to delegate
this to the GPU driver.
Actually seeing all the acpi_video_backlight_use_native()
checks here, I wonder why only have this work with native backlight
control. One step better would be to add thermal_cooling_device
support to the backlight core in:
drivers/video/backlight/backlight.c
Then it will work with any backlight control provider!
Last but not least this code MUST not call
acpi_video_backlight_use_native()
No code other then native GPU drivers must ever call
acpi_video_backlight_use_native(). This special function
not only checks if the native backlight control is the
one which the detection code in drivers/acpi/video_detect.c
has selected, it also signals to video_detect.c that
native GPU backlight control is available.
So by calling this in the AMD PMF code you are now
telling video_detect.c that native GPU backlight control
is available on all systems where AMD PMF runs.
As I already said I really believe the whole cooling
device should be registered somewhere else. But if you
do end up sticking with this then you MUST replace
the acpi_video_backlight_use_native() calls with:
if (acpi_video_get_backlight_type() == acpi_backlight_native) {...}
Regards,
Hans
Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
On 10/18/2023 9:37 PM, Christian König wrote: > Am 18.10.23 um 17:47 schrieb Mario Limonciello: >> On 10/18/2023 08:40, Christian König wrote: >>> >>> >>> Am 18.10.23 um 11:28 schrieb Shyam Sundar S K: On 10/18/2023 2:50 PM, Ilpo Järvinen wrote: > On Wed, 18 Oct 2023, Shyam Sundar S K wrote: > >> In order to provide GPU inputs to TA for the Smart PC solution >> to work, we >> need to have interface between the PMF driver and the AMDGPU >> driver. >> >> Add the initial code path for get interface from AMDGPU. >> >> Co-developed-by: Mario Limonciello >> Signed-off-by: Mario Limonciello >> Signed-off-by: Shyam Sundar S K >> --- >> drivers/gpu/drm/amd/amdgpu/Makefile | 2 + >> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138 >> >> drivers/platform/x86/amd/pmf/Kconfig | 1 + >> drivers/platform/x86/amd/pmf/core.c | 1 + >> drivers/platform/x86/amd/pmf/pmf.h | 3 + >> drivers/platform/x86/amd/pmf/spc.c | 13 +++ >> drivers/platform/x86/amd/pmf/tee-if.c | 26 + >> include/linux/amd-pmf-io.h | 35 ++ >> 9 files changed, 220 insertions(+) >> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c >> create mode 100644 include/linux/amd-pmf-io.h >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile >> b/drivers/gpu/drm/amd/amdgpu/Makefile >> index 384b798a9bad..7fafccefbd7a 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/Makefile >> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile >> @@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o >> amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o >> +amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o >> + >> # add asic specific block >> amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \ >> dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> index a79d53bdbe13..26ffa1b4fe57 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >> @@ -50,6 +50,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c >> new file mode 100644 >> index ..ac981848df50 >> --- /dev/null >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c >> @@ -0,0 +1,138 @@ >> +/* >> + * Copyright 2023 Advanced Micro Devices, Inc. >> + * >> + * Permission is hereby granted, free of charge, to any person >> obtaining a >> + * copy of this software and associated documentation files >> (the "Software"), >> + * to deal in the Software without restriction, including >> without limitation >> + * the rights to use, copy, modify, merge, publish, distribute, >> sublicense, >> + * and/or sell copies of the Software, and to permit persons to >> whom the >> + * Software is furnished to do so, subject to the following >> conditions: >> + * >> + * The above copyright notice and this permission notice shall >> be included in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY >> KIND, EXPRESS OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >> MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO >> EVENT SHALL >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY >> CLAIM, DAMAGES OR >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR >> OTHERWISE, >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR >> THE USE OR >> + * OTHER DEALINGS IN THE SOFTWARE. > This is MIT, right? Add the required SPDX-License-Identifier line > for it > at the top of the file, thank you. > all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license text. So, have retained it to maintain uniformity. >>> >>> Please add the SPDX License Identifier for any file you add. OK. I thought to keep it same like the other files. I will change this to SPDX in the next revision. >>> >>> Apart from that the whole approach of attaching this directly to >>> amdgpu looks extremely questionable to me. >>> >> >> What's the long term outlook for things that are needed directly >> from amdgpu? Is there going to be more besides the backlight and >> the display count/type? > > Yeah, that goes into the same direction as my concern. PMF is an APU only feature and will need inputs from multiple subsystems (in this case its GPU) to send changing system information to PMF TA (Trusted
Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
Am 18.10.23 um 17:47 schrieb Mario Limonciello:
On 10/18/2023 08:40, Christian König wrote:
Am 18.10.23 um 11:28 schrieb Shyam Sundar S K:
On 10/18/2023 2:50 PM, Ilpo Järvinen wrote:
On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
In order to provide GPU inputs to TA for the Smart PC solution to
work, we
need to have interface between the PMF driver and the AMDGPU driver.
Add the initial code path for get interface from AMDGPU.
Co-developed-by: Mario Limonciello
Signed-off-by: Mario Limonciello
Signed-off-by: Shyam Sundar S K
---
drivers/gpu/drm/amd/amdgpu/Makefile | 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138
drivers/platform/x86/amd/pmf/Kconfig | 1 +
drivers/platform/x86/amd/pmf/core.c | 1 +
drivers/platform/x86/amd/pmf/pmf.h | 3 +
drivers/platform/x86/amd/pmf/spc.c | 13 +++
drivers/platform/x86/amd/pmf/tee-if.c | 26 +
include/linux/amd-pmf-io.h | 35 ++
9 files changed, 220 insertions(+)
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
create mode 100644 include/linux/amd-pmf-io.h
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 384b798a9bad..7fafccefbd7a 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
+amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o
+
# add asic specific block
amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \
dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a79d53bdbe13..26ffa1b4fe57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -50,6 +50,7 @@
#include
#include
#include
+#include
#include
#include
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
new file mode 100644
index ..ac981848df50
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
@@ -0,0 +1,138 @@
+/*
+ * Copyright 2023 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person
obtaining a
+ * copy of this software and associated documentation files (the
"Software"),
+ * to deal in the Software without restriction, including without
limitation
+ * the rights to use, copy, modify, merge, publish, distribute,
sublicense,
+ * and/or sell copies of the Software, and to permit persons to
whom the
+ * Software is furnished to do so, subject to the following
conditions:
+ *
+ * The above copyright notice and this permission notice shall be
included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
This is MIT, right? Add the required SPDX-License-Identifier line
for it
at the top of the file, thank you.
all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license
text. So, have retained it to maintain uniformity.
Please add the SPDX License Identifier for any file you add.
Apart from that the whole approach of attaching this directly to
amdgpu looks extremely questionable to me.
What's the long term outlook for things that are needed directly from
amdgpu? Is there going to be more besides the backlight and the
display count/type?
Yeah, that goes into the same direction as my concern.
At least for the display count I suppose one way that it could be
"decoupled" from amdgpu is to use drm_for_each_connector_iter to
iterate all the connectors and then count the connected ones.
And what if the number of connected displays change? How is amdgpu
supposed to signal events like that?
This whole solution doesn't looks well thought through.
Regards,
Christian.
Regards,
Christian.
+ *
+ * Author: Shyam Sundar S K
+ *
+ */
Remove the extra empty line at the end of the comment.
I just took the standard template for all the gpu files. Is that OK to
retain the blank line?
If not, I can remove it in the next version.
Rest all remarks I will address.
Thanks,
Shyam
+
+#include
+#include "amdgpu.h"
+
+int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
+{
+ struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
+ struct drm_mode_config *mode_config = _dev->mode_config;
+ struct amdgpu_device *adev = drm_to_adev(drm_dev);
+
Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
On 10/18/2023 08:40, Christian König wrote:
Am 18.10.23 um 11:28 schrieb Shyam Sundar S K:
On 10/18/2023 2:50 PM, Ilpo Järvinen wrote:
On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
In order to provide GPU inputs to TA for the Smart PC solution to
work, we
need to have interface between the PMF driver and the AMDGPU driver.
Add the initial code path for get interface from AMDGPU.
Co-developed-by: Mario Limonciello
Signed-off-by: Mario Limonciello
Signed-off-by: Shyam Sundar S K
---
drivers/gpu/drm/amd/amdgpu/Makefile | 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138
drivers/platform/x86/amd/pmf/Kconfig | 1 +
drivers/platform/x86/amd/pmf/core.c | 1 +
drivers/platform/x86/amd/pmf/pmf.h | 3 +
drivers/platform/x86/amd/pmf/spc.c | 13 +++
drivers/platform/x86/amd/pmf/tee-if.c | 26 +
include/linux/amd-pmf-io.h | 35 ++
9 files changed, 220 insertions(+)
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
create mode 100644 include/linux/amd-pmf-io.h
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 384b798a9bad..7fafccefbd7a 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
+amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o
+
# add asic specific block
amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \
dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a79d53bdbe13..26ffa1b4fe57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -50,6 +50,7 @@
#include
#include
#include
+#include
#include
#include
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
new file mode 100644
index ..ac981848df50
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
@@ -0,0 +1,138 @@
+/*
+ * Copyright 2023 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person
obtaining a
+ * copy of this software and associated documentation files (the
"Software"),
+ * to deal in the Software without restriction, including without
limitation
+ * the rights to use, copy, modify, merge, publish, distribute,
sublicense,
+ * and/or sell copies of the Software, and to permit persons to
whom the
+ * Software is furnished to do so, subject to the following
conditions:
+ *
+ * The above copyright notice and this permission notice shall be
included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
This is MIT, right? Add the required SPDX-License-Identifier line for it
at the top of the file, thank you.
all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license
text. So, have retained it to maintain uniformity.
Please add the SPDX License Identifier for any file you add.
Apart from that the whole approach of attaching this directly to amdgpu
looks extremely questionable to me.
What's the long term outlook for things that are needed directly from
amdgpu? Is there going to be more besides the backlight and the display
count/type?
At least for the display count I suppose one way that it could be
"decoupled" from amdgpu is to use drm_for_each_connector_iter to iterate
all the connectors and then count the connected ones.
Regards,
Christian.
+ *
+ * Author: Shyam Sundar S K
+ *
+ */
Remove the extra empty line at the end of the comment.
I just took the standard template for all the gpu files. Is that OK to
retain the blank line?
If not, I can remove it in the next version.
Rest all remarks I will address.
Thanks,
Shyam
+
+#include
+#include "amdgpu.h"
+
+int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
+{
+ struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
+ struct drm_mode_config *mode_config = _dev->mode_config;
+ struct amdgpu_device *adev = drm_to_adev(drm_dev);
+ struct drm_connector_list_iter iter;
+ struct drm_connector *connector;
+ int i = 0;
+
+ /* Reset the count to zero */
+ pmf->display_count = 0;
+ if (!(adev->flags & AMD_IS_APU)) {
+ DRM_ERROR("PMF-AMDGPU interface not supported\n");
+ return -ENODEV;
+ }
+
+
Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
Am 18.10.23 um 11:28 schrieb Shyam Sundar S K:
On 10/18/2023 2:50 PM, Ilpo Järvinen wrote:
On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
In order to provide GPU inputs to TA for the Smart PC solution to work, we
need to have interface between the PMF driver and the AMDGPU driver.
Add the initial code path for get interface from AMDGPU.
Co-developed-by: Mario Limonciello
Signed-off-by: Mario Limonciello
Signed-off-by: Shyam Sundar S K
---
drivers/gpu/drm/amd/amdgpu/Makefile | 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138
drivers/platform/x86/amd/pmf/Kconfig| 1 +
drivers/platform/x86/amd/pmf/core.c | 1 +
drivers/platform/x86/amd/pmf/pmf.h | 3 +
drivers/platform/x86/amd/pmf/spc.c | 13 +++
drivers/platform/x86/amd/pmf/tee-if.c | 26 +
include/linux/amd-pmf-io.h | 35 ++
9 files changed, 220 insertions(+)
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
create mode 100644 include/linux/amd-pmf-io.h
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 384b798a9bad..7fafccefbd7a 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
+amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o
+
# add asic specific block
amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \
dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a79d53bdbe13..26ffa1b4fe57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -50,6 +50,7 @@
#include
#include
#include
+#include
#include
#include
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
new file mode 100644
index ..ac981848df50
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
@@ -0,0 +1,138 @@
+/*
+ * Copyright 2023 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
This is MIT, right? Add the required SPDX-License-Identifier line for it
at the top of the file, thank you.
all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license
text. So, have retained it to maintain uniformity.
Please add the SPDX License Identifier for any file you add.
Apart from that the whole approach of attaching this directly to amdgpu
looks extremely questionable to me.
Regards,
Christian.
+ *
+ * Author: Shyam Sundar S K
+ *
+ */
Remove the extra empty line at the end of the comment.
I just took the standard template for all the gpu files. Is that OK to
retain the blank line?
If not, I can remove it in the next version.
Rest all remarks I will address.
Thanks,
Shyam
+
+#include
+#include "amdgpu.h"
+
+int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
+{
+ struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
+ struct drm_mode_config *mode_config = _dev->mode_config;
+ struct amdgpu_device *adev = drm_to_adev(drm_dev);
+ struct drm_connector_list_iter iter;
+ struct drm_connector *connector;
+ int i = 0;
+
+ /* Reset the count to zero */
+ pmf->display_count = 0;
+ if (!(adev->flags & AMD_IS_APU)) {
+ DRM_ERROR("PMF-AMDGPU interface not supported\n");
+ return -ENODEV;
+ }
+
+ mutex_lock(_config->mutex);
+ drm_connector_list_iter_begin(drm_dev, );
+ drm_for_each_connector_iter(connector, ) {
+ if (connector->status == connector_status_connected)
+ pmf->display_count++;
+ if (connector->status != pmf->con_status[i])
+ pmf->con_status[i] =
Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
> In order to provide GPU inputs to TA for the Smart PC solution to work, we
> need to have interface between the PMF driver and the AMDGPU driver.
>
> Add the initial code path for get interface from AMDGPU.
>
> Co-developed-by: Mario Limonciello
> Signed-off-by: Mario Limonciello
> Signed-off-by: Shyam Sundar S K
> ---
> drivers/gpu/drm/amd/amdgpu/Makefile | 2 +
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138
> drivers/platform/x86/amd/pmf/Kconfig| 1 +
> drivers/platform/x86/amd/pmf/core.c | 1 +
> drivers/platform/x86/amd/pmf/pmf.h | 3 +
> drivers/platform/x86/amd/pmf/spc.c | 13 +++
> drivers/platform/x86/amd/pmf/tee-if.c | 26 +
> include/linux/amd-pmf-io.h | 35 ++
> 9 files changed, 220 insertions(+)
> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> create mode 100644 include/linux/amd-pmf-io.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
> b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 384b798a9bad..7fafccefbd7a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
>
> amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
>
> +amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o
> +
> # add asic specific block
> amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \
> dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index a79d53bdbe13..26ffa1b4fe57 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -50,6 +50,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> new file mode 100644
> index ..ac981848df50
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
> @@ -0,0 +1,138 @@
> +/*
> + * Copyright 2023 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
This is MIT, right? Add the required SPDX-License-Identifier line for it
at the top of the file, thank you.
> + *
> + * Author: Shyam Sundar S K
> + *
> + */
Remove the extra empty line at the end of the comment.
> +
> +#include
> +#include "amdgpu.h"
> +
> +int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
> +{
> + struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
> + struct drm_mode_config *mode_config = _dev->mode_config;
> + struct amdgpu_device *adev = drm_to_adev(drm_dev);
> + struct drm_connector_list_iter iter;
> + struct drm_connector *connector;
> + int i = 0;
> +
> + /* Reset the count to zero */
> + pmf->display_count = 0;
> + if (!(adev->flags & AMD_IS_APU)) {
> + DRM_ERROR("PMF-AMDGPU interface not supported\n");
> + return -ENODEV;
> + }
> +
> + mutex_lock(_config->mutex);
> + drm_connector_list_iter_begin(drm_dev, );
> + drm_for_each_connector_iter(connector, ) {
> + if (connector->status == connector_status_connected)
> + pmf->display_count++;
> + if (connector->status != pmf->con_status[i])
> + pmf->con_status[i] = connector->status;
> + if (connector->connector_type != pmf->connector_type[i])
> + pmf->connector_type[i] = connector->connector_type;
> +
> + i++;
> + if (i >= MAX_SUPPORTED)
> + break;
> + }
> + drm_connector_list_iter_end();
> + mutex_unlock(_config->mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
> +
> +static int amd_pmf_gpu_get_cur_state(struct
Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
On Wed, 18 Oct 2023, Shyam Sundar S K wrote: > On 10/18/2023 2:50 PM, Ilpo Järvinen wrote: > > On Wed, 18 Oct 2023, Shyam Sundar S K wrote: > > > >> In order to provide GPU inputs to TA for the Smart PC solution to work, we > >> need to have interface between the PMF driver and the AMDGPU driver. > >> > >> Add the initial code path for get interface from AMDGPU. > >> > >> Co-developed-by: Mario Limonciello > >> Signed-off-by: Mario Limonciello > >> Signed-off-by: Shyam Sundar S K > >> --- > >> drivers/gpu/drm/amd/amdgpu/Makefile | 2 + > >> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + > >> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138 > >> drivers/platform/x86/amd/pmf/Kconfig| 1 + > >> drivers/platform/x86/amd/pmf/core.c | 1 + > >> drivers/platform/x86/amd/pmf/pmf.h | 3 + > >> drivers/platform/x86/amd/pmf/spc.c | 13 +++ > >> drivers/platform/x86/amd/pmf/tee-if.c | 26 + > >> include/linux/amd-pmf-io.h | 35 ++ > >> 9 files changed, 220 insertions(+) > >> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c > >> create mode 100644 include/linux/amd-pmf-io.h > >> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile > >> b/drivers/gpu/drm/amd/amdgpu/Makefile > >> index 384b798a9bad..7fafccefbd7a 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/Makefile > >> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile > >> @@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o > >> > >> amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o > >> > >> +amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o > >> + > >> # add asic specific block > >> amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \ > >>dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > >> index a79d53bdbe13..26ffa1b4fe57 100644 > >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > >> @@ -50,6 +50,7 @@ > >> #include > >> #include > >> #include > >> +#include > >> > >> #include > >> #include > >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c > >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c > >> new file mode 100644 > >> index ..ac981848df50 > >> --- /dev/null > >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c > >> @@ -0,0 +1,138 @@ > >> +/* > >> + * Copyright 2023 Advanced Micro Devices, Inc. > >> + * > >> + * Permission is hereby granted, free of charge, to any person obtaining a > >> + * copy of this software and associated documentation files (the > >> "Software"), > >> + * to deal in the Software without restriction, including without > >> limitation > >> + * the rights to use, copy, modify, merge, publish, distribute, > >> sublicense, > >> + * and/or sell copies of the Software, and to permit persons to whom the > >> + * Software is furnished to do so, subject to the following conditions: > >> + * > >> + * The above copyright notice and this permission notice shall be > >> included in > >> + * all copies or substantial portions of the Software. > >> + * > >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > >> EXPRESS OR > >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > >> MERCHANTABILITY, > >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT > >> SHALL > >> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES > >> OR > >> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > >> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > >> + * OTHER DEALINGS IN THE SOFTWARE. > > > > This is MIT, right? Add the required SPDX-License-Identifier line for it > > at the top of the file, thank you. > > > > all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license > text. So, have retained it to maintain uniformity. What does adding SPDX identifier line at the top of the file take away from that? I'd be fine if you want to add the identifier line to all of them too to keep them identical. > >> + * > >> + * Author: Shyam Sundar S K > >> + * > >> + */ > > > > Remove the extra empty line at the end of the comment. > > > > I just took the standard template for all the gpu files. Is that OK to > retain the blank line? > > If not, I can remove it in the next version. I don't want to fight over a blank line when you insist on keeping it :-). -- i.
Re: [PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
On 10/18/2023 2:50 PM, Ilpo Järvinen wrote:
> On Wed, 18 Oct 2023, Shyam Sundar S K wrote:
>
>> In order to provide GPU inputs to TA for the Smart PC solution to work, we
>> need to have interface between the PMF driver and the AMDGPU driver.
>>
>> Add the initial code path for get interface from AMDGPU.
>>
>> Co-developed-by: Mario Limonciello
>> Signed-off-by: Mario Limonciello
>> Signed-off-by: Shyam Sundar S K
>> ---
>> drivers/gpu/drm/amd/amdgpu/Makefile | 2 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
>> drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138
>> drivers/platform/x86/amd/pmf/Kconfig| 1 +
>> drivers/platform/x86/amd/pmf/core.c | 1 +
>> drivers/platform/x86/amd/pmf/pmf.h | 3 +
>> drivers/platform/x86/amd/pmf/spc.c | 13 +++
>> drivers/platform/x86/amd/pmf/tee-if.c | 26 +
>> include/linux/amd-pmf-io.h | 35 ++
>> 9 files changed, 220 insertions(+)
>> create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> create mode 100644 include/linux/amd-pmf-io.h
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
>> b/drivers/gpu/drm/amd/amdgpu/Makefile
>> index 384b798a9bad..7fafccefbd7a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
>> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
>> @@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
>>
>> amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
>>
>> +amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o
>> +
>> # add asic specific block
>> amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \
>> dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index a79d53bdbe13..26ffa1b4fe57 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -50,6 +50,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> new file mode 100644
>> index ..ac981848df50
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
>> @@ -0,0 +1,138 @@
>> +/*
>> + * Copyright 2023 Advanced Micro Devices, Inc.
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the
>> "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>
> This is MIT, right? Add the required SPDX-License-Identifier line for it
> at the top of the file, thank you.
>
all the files in drivers/gpu/drm/amd/amdgpu/* carry the same license
text. So, have retained it to maintain uniformity.
>> + *
>> + * Author: Shyam Sundar S K
>> + *
>> + */
>
> Remove the extra empty line at the end of the comment.
>
I just took the standard template for all the gpu files. Is that OK to
retain the blank line?
If not, I can remove it in the next version.
Rest all remarks I will address.
Thanks,
Shyam
>> +
>> +#include
>> +#include "amdgpu.h"
>> +
>> +int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
>> +{
>> +struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
>> +struct drm_mode_config *mode_config = _dev->mode_config;
>> +struct amdgpu_device *adev = drm_to_adev(drm_dev);
>> +struct drm_connector_list_iter iter;
>> +struct drm_connector *connector;
>> +int i = 0;
>> +
>> +/* Reset the count to zero */
>> +pmf->display_count = 0;
>> +if (!(adev->flags & AMD_IS_APU)) {
>> +DRM_ERROR("PMF-AMDGPU interface not supported\n");
>> +return -ENODEV;
>> +}
>> +
>> +mutex_lock(_config->mutex);
>> +drm_connector_list_iter_begin(drm_dev, );
>> +drm_for_each_connector_iter(connector, ) {
>> +if (connector->status == connector_status_connected)
>> +pmf->display_count++;
>> +if (connector->status != pmf->con_status[i])
>> +pmf->con_status[i] =
[PATCH v4 13/17] platform/x86/amd/pmf: Add PMF-AMDGPU get interface
In order to provide GPU inputs to TA for the Smart PC solution to work, we
need to have interface between the PMF driver and the AMDGPU driver.
Add the initial code path for get interface from AMDGPU.
Co-developed-by: Mario Limonciello
Signed-off-by: Mario Limonciello
Signed-off-by: Shyam Sundar S K
---
drivers/gpu/drm/amd/amdgpu/Makefile | 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c | 138
drivers/platform/x86/amd/pmf/Kconfig| 1 +
drivers/platform/x86/amd/pmf/core.c | 1 +
drivers/platform/x86/amd/pmf/pmf.h | 3 +
drivers/platform/x86/amd/pmf/spc.c | 13 +++
drivers/platform/x86/amd/pmf/tee-if.c | 26 +
include/linux/amd-pmf-io.h | 35 ++
9 files changed, 220 insertions(+)
create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
create mode 100644 include/linux/amd-pmf-io.h
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile
b/drivers/gpu/drm/amd/amdgpu/Makefile
index 384b798a9bad..7fafccefbd7a 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -86,6 +86,8 @@ amdgpu-$(CONFIG_PROC_FS) += amdgpu_fdinfo.o
amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
+amdgpu-$(CONFIG_AMD_PMF) += amdgpu_pmf.o
+
# add asic specific block
amdgpu-$(CONFIG_DRM_AMDGPU_CIK)+= cik.o cik_ih.o \
dce_v8_0.o gfx_v7_0.o cik_sdma.o uvd_v4_2.o vce_v2_0.o
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a79d53bdbe13..26ffa1b4fe57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -50,6 +50,7 @@
#include
#include
#include
+#include
#include
#include
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
new file mode 100644
index ..ac981848df50
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmf.c
@@ -0,0 +1,138 @@
+/*
+ * Copyright 2023 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Author: Shyam Sundar S K
+ *
+ */
+
+#include
+#include "amdgpu.h"
+
+int amd_pmf_get_gfx_data(struct amd_gpu_pmf_data *pmf)
+{
+ struct drm_device *drm_dev = pci_get_drvdata(pmf->gpu_dev);
+ struct drm_mode_config *mode_config = _dev->mode_config;
+ struct amdgpu_device *adev = drm_to_adev(drm_dev);
+ struct drm_connector_list_iter iter;
+ struct drm_connector *connector;
+ int i = 0;
+
+ /* Reset the count to zero */
+ pmf->display_count = 0;
+ if (!(adev->flags & AMD_IS_APU)) {
+ DRM_ERROR("PMF-AMDGPU interface not supported\n");
+ return -ENODEV;
+ }
+
+ mutex_lock(_config->mutex);
+ drm_connector_list_iter_begin(drm_dev, );
+ drm_for_each_connector_iter(connector, ) {
+ if (connector->status == connector_status_connected)
+ pmf->display_count++;
+ if (connector->status != pmf->con_status[i])
+ pmf->con_status[i] = connector->status;
+ if (connector->connector_type != pmf->connector_type[i])
+ pmf->connector_type[i] = connector->connector_type;
+
+ i++;
+ if (i >= MAX_SUPPORTED)
+ break;
+ }
+ drm_connector_list_iter_end();
+ mutex_unlock(_config->mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(amd_pmf_get_gfx_data);
+
+static int amd_pmf_gpu_get_cur_state(struct thermal_cooling_device
*cooling_dev,
+unsigned long *state)
+{
+ struct backlight_device *bd;
+
+ if (!acpi_video_backlight_use_native())
+ return -ENODEV;
+
+ bd = backlight_device_get_by_type(BACKLIGHT_RAW);
+ if (!bd)
+ return -ENODEV;
+
+ *state = backlight_get_brightness(bd);
+
+ return 0;
+}
+
