Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()

[Rafael J. Wysocki]

On Thu, May 16, 2024 at 7:35 PM Steven Rostedt  wrote:
>
> From: "Steven Rostedt (Google)" 
>
> [
>This is a treewide change. I will likely re-create this patch again in
>the second week of the merge window of v6.10 and submit it then. Hoping
>to keep the conflicts that it will cause to a minimum.
> ]
>
> With the rework of how the __string() handles dynamic strings where it
> saves off the source string in field in the helper structure[1], the
> assignment of that value to the trace event field is stored in the helper
> value and does not need to be passed in again.
>
> This means that with:
>
>   __string(field, mystring)
>
> Which use to be assigned with __assign_str(field, mystring), no longer
> needs the second parameter and it is unused. With this, __assign_str()
> will now only get a single parameter.
>
> There's over 700 users of __assign_str() and because coccinelle does not
> handle the TRACE_EVENT() macro I ended up using the following sed script:
>
>   git grep -l __assign_str | while read a ; do
>   sed -e 's/\(__assign_str([^,]*[^ ,]\) *,[^;]*/\1)/' $a > /tmp/test-file;
>   mv /tmp/test-file $a;
>   done
>
> I then searched for __assign_str() that did not end with ';' as those
> were multi line assignments that the sed script above would fail to catch.
>
> Note, the same updates will need to be done for:
>
>   __assign_str_len()
>   __assign_rel_str()
>   __assign_rel_str_len()
>
> I tested this with both an allmodconfig and an allyesconfig (build only for 
> both).
>
> [1] 
> https://lore.kernel.org/linux-trace-kernel/2024011442.634192...@goodmis.org/
>
> Cc: Masami Hiramatsu 
> Cc: Mathieu Desnoyers 
> Cc: Linus Torvalds 
> Cc: Julia Lawall 
> Signed-off-by: Steven Rostedt (Google) 

Acked-by: Rafael J. Wysocki  # for thermal

Re: [PATCH v3 1/5] ACPI: video: Handle fetching EDID that is longer than 256 bytes

[Rafael J. Wysocki]

On Fri, Feb 2, 2024 at 5:09 PM Mario Limonciello
 wrote:
>
> On 2/2/2024 10:07, Rafael J. Wysocki wrote:
> > On Thu, Feb 1, 2024 at 11:11 PM Mario Limonciello
> >  wrote:
> >>
> >> The ACPI specification allows for an EDID to be up to 512 bytes but
> >> the _DDC EDID fetching code will only try up to 256 bytes.
> >>
> >> Modify the code to instead start at 512 bytes and work it's way
> >> down instead.
> >>
> >> As _DDC is now called up to 4 times on a machine debugging messages
> >> are noisier than necessary.  Decrease from info to debug.
> >>
> >> Link: 
> >> https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/Apx_B_Video_Extensions/output-device-specific-methods.html#ddc-return-the-edid-for-this-device
> >> Signed-off-by: Mario Limonciello 
> >
> > Acked-by: Rafael J. Wysocki 
> >
> > or I can apply it if that's preferred.
>
> Thanks!
>
> I think go ahead and apply this one to your -next tree.

Applied now.

Barring any issues with it, It will get into linux-next in a couple of days.

Thanks!

Re: [PATCH v3 1/5] ACPI: video: Handle fetching EDID that is longer than 256 bytes

[Rafael J. Wysocki]

On Thu, Feb 1, 2024 at 11:11 PM Mario Limonciello
 wrote:
>
> The ACPI specification allows for an EDID to be up to 512 bytes but
> the _DDC EDID fetching code will only try up to 256 bytes.
>
> Modify the code to instead start at 512 bytes and work it's way
> down instead.
>
> As _DDC is now called up to 4 times on a machine debugging messages
> are noisier than necessary.  Decrease from info to debug.
>
> Link: 
> https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/Apx_B_Video_Extensions/output-device-specific-methods.html#ddc-return-the-edid-for-this-device
> Signed-off-by: Mario Limonciello 

Acked-by: Rafael J. Wysocki 

or I can apply it if that's preferred.

Thanks!

> ---
> v1->v2:
>  * Use for loop for acpi_video_get_edid()
>  * Use one of Rafael's suggestions for acpi_video_device_EDID()
>  * Decrease message level too
> ---
>  drivers/acpi/acpi_video.c | 25 +
>  1 file changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index 4afdda9db019..3bfd013e09d2 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -625,12 +625,9 @@ acpi_video_device_EDID(struct acpi_video_device *device,
>
> if (!device)
> return -ENODEV;
> -   if (length == 128)
> -   arg0.integer.value = 1;
> -   else if (length == 256)
> -   arg0.integer.value = 2;
> -   else
> +   if (!length || (length % 128))
> return -EINVAL;
> +   arg0.integer.value = length / 128;
>
> status = acpi_evaluate_object(device->dev->handle, "_DDC", , 
> );
> if (ACPI_FAILURE(status))
> @@ -641,7 +638,8 @@ acpi_video_device_EDID(struct acpi_video_device *device,
> if (obj && obj->type == ACPI_TYPE_BUFFER)
> *edid = obj;
> else {
> -   acpi_handle_info(device->dev->handle, "Invalid _DDC data\n");
> +   acpi_handle_debug(device->dev->handle,
> +"Invalid _DDC data for length %ld\n", 
> length);
> status = -EFAULT;
> kfree(obj);
> }
> @@ -1447,7 +1445,6 @@ int acpi_video_get_edid(struct acpi_device *device, int 
> type, int device_id,
>
> for (i = 0; i < video->attached_count; i++) {
> video_device = video->attached_array[i].bind_info;
> -   length = 256;
>
> if (!video_device)
> continue;
> @@ -1478,18 +1475,14 @@ int acpi_video_get_edid(struct acpi_device *device, 
> int type, int device_id,
> continue;
> }
>
> -   status = acpi_video_device_EDID(video_device, , 
> length);
> -
> -   if (ACPI_FAILURE(status) || !buffer ||
> -   buffer->type != ACPI_TYPE_BUFFER) {
> -   length = 128;
> +   for (length = 512; length > 0; length -= 128) {
> status = acpi_video_device_EDID(video_device, ,
> length);
> -   if (ACPI_FAILURE(status) || !buffer ||
> -   buffer->type != ACPI_TYPE_BUFFER) {
> -   continue;
> -   }
> +   if (ACPI_SUCCESS(status))
> +   break;
> }
> +   if (!length)
> +   continue;
>
> *edid = buffer->buffer.pointer;
> return length;
> --
> 2.34.1
>

Re: [PATCH 1/2] ACPI: video: Handle fetching EDID that is longer than 256 bytes

[Rafael J. Wysocki]

On Fri, Jan 26, 2024 at 7:55 PM Mario Limonciello
 wrote:
>
> The ACPI specification allows for an EDID to be up to 512 bytes but
> the _DDC EDID fetching code will only try up to 256 bytes.
>
> Modify the code to instead start at 512 bytes and work it's way
> down instead.
>
> Link: 
> https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/Apx_B_Video_Extensions/output-device-specific-methods.html#ddc-return-the-edid-for-this-device
> Signed-off-by: Mario Limonciello 
> ---
>  drivers/acpi/acpi_video.c | 23 ---
>  1 file changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index 62f4364e4460..b3b15dd4755d 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -624,6 +624,10 @@ acpi_video_device_EDID(struct acpi_video_device *device,
> arg0.integer.value = 1;
> else if (length == 256)
> arg0.integer.value = 2;
> +   else if (length == 384)
> +   arg0.integer.value = 3;
> +   else if (length == 512)
> +   arg0.integer.value = 4;

It looks like switch () would be somewhat better.

Or maybe even

arg0.integer.value = length / 128;

The validation could be added too:

if (arg0.integer.value > 4 || arg0.integer.value * 128 != length)
return -EINVAL;

but it is pointless, because the caller is never passing an invalid
number to it AFAICS.

> else
> return -EINVAL;
>
> @@ -1443,7 +1447,7 @@ int acpi_video_get_edid(struct acpi_device *device, int 
> type, int device_id,
>
> for (i = 0; i < video->attached_count; i++) {
> video_device = video->attached_array[i].bind_info;
> -   length = 256;
> +   length = 512;
>
> if (!video_device)
> continue;
> @@ -1478,13 +1482,18 @@ int acpi_video_get_edid(struct acpi_device *device, 
> int type, int device_id,
>
> if (ACPI_FAILURE(status) || !buffer ||
> buffer->type != ACPI_TYPE_BUFFER) {
> -   length = 128;
> -   status = acpi_video_device_EDID(video_device, ,
> -   length);
> -   if (ACPI_FAILURE(status) || !buffer ||
> -   buffer->type != ACPI_TYPE_BUFFER) {
> -   continue;
> +   while (length) {

I would prefer a do {} while () loop here, which could include the
first invocation of acpi_video_device_EDID() too (and reduce code
duplication a bit).

> +   length -= 128;
> +   status = acpi_video_device_EDID(video_device, 
> ,
> +   length);

No line break, please.

> +   if (ACPI_FAILURE(status) || !buffer ||
> +   buffer->type != ACPI_TYPE_BUFFER) {
> +   continue;
> +   }
> +   break;
> }
> +   if (!length)
> +   continue;
> }
>
> *edid = buffer->buffer.pointer;
> --

Re: [V11 1/8] ACPI: Add support for AMD ACPI based Wifi band RFI mitigation feature

[Rafael J. Wysocki]

On Thu, Aug 31, 2023 at 8:21 AM Evan Quan  wrote:
>
> Due to electrical and mechanical constraints in certain platform designs
> there may be likely interference of relatively high-powered harmonics of
> the (G-)DDR memory clocks with local radio module frequency bands used
> by Wifi 6/6e/7.
>
> To mitigate this, AMD has introduced a mechanism that devices can use to
> notify active use of particular frequencies so that other devices can make
> relative internal adjustments as necessary to avoid this resonance.

The changelog is only marginally useful IMV.

It doesn't even mention the role of ACPI in all this, so it is quite
unclear what the patch is all about, why it does what it does and what
is actually done in it.

It is also unclear why this code is put into drivers/acpi/, which
should be explained.

> Signed-off-by: Evan Quan 
> --
> v10->v11:
>   - fix typo(Simon)
> ---
>  drivers/acpi/Kconfig  |  17 ++
>  drivers/acpi/Makefile |   2 +
>  drivers/acpi/amd_wbrf.c   | 414 ++
>  include/linux/acpi_amd_wbrf.h | 140 
>  4 files changed, 573 insertions(+)
>  create mode 100644 drivers/acpi/amd_wbrf.c
>  create mode 100644 include/linux/acpi_amd_wbrf.h
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 00dd309b6682..a092ea72d152 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -594,6 +594,23 @@ config ACPI_PRMT
>   substantially increase computational overhead related to the
>   initialization of some server systems.
>
> +config WBRF_AMD_ACPI
> +   bool "ACPI based WBRF mechanism introduced by AMD"
> +   depends on ACPI
> +   default n
> +   help
> + Wifi band RFI mitigation mechanism allows multiple drivers from
> + different domains to notify the frequencies in use so that hardware
> + can be reconfigured to avoid harmonic conflicts.

So drivers can notify the platform firmware IIUC, but that is not
really clear from the above.  I'm not even sure what the phrase
"notify the frequencies in use" is supposed to mean.

> +
> + AMD has introduced an ACPI based mechanism to support WBRF for some
> + platforms with AMD dGPU and WLAN. This needs support from BIOS 
> equipped
> + with necessary AML implementations and dGPU firmwares.
> +
> + Before enabling this ACPI based mechanism, it is suggested to 
> confirm
> + with the hardware designer/provider first whether your platform
> + equipped with necessary BIOS and firmwares.

No, this doesn't work.

Say you are a distro and you want to supply all of your users with the
same binary kernel image.  What are you expected to do to address the
above?

> +
>  endif  # ACPI
>
>  config X86_PM_TIMER
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index eaa09bf52f17..a3d2f259d0a5 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -132,3 +132,5 @@ obj-$(CONFIG_ARM64) += arm64/
>  obj-$(CONFIG_ACPI_VIOT)+= viot.o
>
>  obj-$(CONFIG_RISCV)+= riscv/
> +
> +obj-$(CONFIG_WBRF_AMD_ACPI)+= amd_wbrf.o
> diff --git a/drivers/acpi/amd_wbrf.c b/drivers/acpi/amd_wbrf.c
> new file mode 100644
> index ..8ee0e2977a30
> --- /dev/null
> +++ b/drivers/acpi/amd_wbrf.c
> @@ -0,0 +1,414 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Wifi Band Exclusion Interface (AMD ACPI Implementation)
> + * Copyright (C) 2023 Advanced Micro Devices
> + *

Please document the code in this file at least basically.

You don't even explain what Wifi Band Exclusion means.

The OS-firmware interface that this code is based on should be
described here or a link to its description should be provided at the
very least.

As it is now, one needs to reverse engineer the patch in order to get
any idea about how this interface is designed.

> + */
> +
> +#include 
> +#include 
> +
> +#define ACPI_AMD_WBRF_METHOD   "\\WBRF"
> +
> +/*
> + * Functions bit vector for WBRF method
> + *
> + * Bit 0: Supported for any functions other than function 0.
> + * Bit 1: Function 1 (Add / Remove frequency) is supported.
> + * Bit 2: Function 2 (Get frequency list) is supported.
> + */

Without any additional information, the comment above is meaningless.

> +#define WBRF_ENABLED   0x0
> +#define WBRF_RECORD0x1
> +#define WBRF_RETRIEVE  0x2
> +
> +/* record actions */
> +#define WBRF_RECORD_ADD0x0
> +#define WBRF_RECORD_REMOVE 0x1
> +
> +#define WBRF_REVISION  0x1
> +
> +/*
> + * The data structure used for WBRF_RETRIEVE is not naturally aligned.
> + * And unfortunately the design has been settled down.
> + */
> +struct amd_wbrf_ranges_out {
> +   u32 num_of_ranges;
> +   struct exclusion_range  band_list[MAX_NUM_OF_WBRF_RANGES];
> +} __packed;
> +
> +static const guid_t wifi_acpi_dsm_guid =
> +   

Re: [V9 2/9] drivers core: add ACPI based WBRF mechanism introduced by AMD

[Rafael J. Wysocki]

On Fri, Aug 18, 2023 at 5:27 AM Evan Quan  wrote:
>
> AMD has introduced an ACPI based mechanism to support WBRF for some
> platforms with AMD dGPU + WLAN. This needs support from BIOS equipped
> with necessary AML implementations and dGPU firmwares.

This needs a problem statement in the first place: What exactly caused
AMD to come up with this design?

> For those systems without the ACPI mechanism and developing solutions,
> user can use/fall-back the generic WBRF solution for diagnosing potential
> interference issues.
>
> And for the platform which does not equip with the necessary AMD ACPI
> implementations but with CONFIG_WBRF_AMD_ACPI built as 'y', it will
> fall back to generic WBRF solution if the `wbrf` is set as "on".

OK, so I suppose that the patch implements support for the AMD WBRF
firmware interface?  That needs to be stated somewhere.

>From patch reverse-engineering it looks like the generic WBRF code is
updated by it to hook up to the ACPI implementation if supported.  If
my understanding is correct, it would be nice to state that in the
changelog too.

> Co-developed-by: Mario Limonciello 
> Signed-off-by: Mario Limonciello 
> Co-developed-by: Evan Quan 
> Signed-off-by: Evan Quan 
> --
> v4->v5:
>   - promote this to be a more generic solution with input argument taking
> `struct device` and provide better scalability to support non-ACPI
> scenarios(Andrew)
>   - update the APIs naming and some other minor fixes(Rafael)
> v5->v6:
>   - make the code more readable and some other fixes(Andrew)
> v6->v8:
>   - drop CONFIG_WBRF_GENERIC(Mario)
>   - add `wbrf` kernel parameter for policy control(Mario)
> v8->v9:
>   - correct some coding style(Simon)
> ---
>  drivers/acpi/Makefile |   2 +
>  drivers/acpi/amd_wbrf.c   | 294 ++
>  drivers/base/Kconfig  |  20 +++
>  drivers/base/wbrf.c   | 135 +---
>  include/linux/acpi_amd_wbrf.h |  25 +++
>  5 files changed, 452 insertions(+), 24 deletions(-)
>  create mode 100644 drivers/acpi/amd_wbrf.c
>  create mode 100644 include/linux/acpi_amd_wbrf.h
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 3fc5a0d54f6e..9185d16e4495 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -133,3 +133,5 @@ obj-$(CONFIG_ARM64) += arm64/
>  obj-$(CONFIG_ACPI_VIOT)+= viot.o
>
>  obj-$(CONFIG_RISCV)+= riscv/
> +
> +obj-$(CONFIG_WBRF_AMD_ACPI)+= amd_wbrf.o
> diff --git a/drivers/acpi/amd_wbrf.c b/drivers/acpi/amd_wbrf.c
> new file mode 100644
> index ..0e46de3dfac7
> --- /dev/null
> +++ b/drivers/acpi/amd_wbrf.c
> @@ -0,0 +1,294 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Wifi Band Exclusion Interface (AMD ACPI Implementation)
> + * Copyright (C) 2023 Advanced Micro Devices
> + *

It would be nice to have a description of the firmware interface here,
at least in general terms.

In particular, the terminology used throughout the code must be explained.

Without it, qualifying the validity and/or usefulness of the code is
rather hard.

> + */
> +
> +#include 
> +#include 
> +
> +#define ACPI_AMD_WBRF_METHOD   "\\WBRF"
> +
> +/*
> + * Functions bit vector for WBRF method
> + *
> + * Bit 0: Supported for any functions other than function 0.
> + * Bit 1: Function 1 (Add / Remove frequency) is supported.
> + * Bit 2: Function 2 (Get frequency list) is supported.
> + */
> +#define WBRF_ENABLED   0x0
> +#define WBRF_RECORD0x1
> +#define WBRF_RETRIEVE  0x2
> +
> +/* record actions */
> +#define WBRF_RECORD_ADD0x0
> +#define WBRF_RECORD_REMOVE 0x1
> +
> +#define WBRF_REVISION  0x1
> +
> +/*
> + * The data structure used for WBRF_RETRIEVE is not natually aligned.

"naturally"

> + * And unfortunately the design has been settled down.
> + */
> +struct amd_wbrf_ranges_out {
> +   u32 num_of_ranges;
> +   struct exclusion_range  band_list[MAX_NUM_OF_WBRF_RANGES];
> +} __packed;
> +
> +static const guid_t wifi_acpi_dsm_guid =
> +   GUID_INIT(0x7b7656cf, 0xdc3d, 0x4c1c,
> + 0x83, 0xe9, 0x66, 0xe7, 0x21, 0xde, 0x30, 0x70);
> +
> +static int wbrf_dsm(struct acpi_device *adev, u8 fn,
> +   union acpi_object *argv4,
> +   union acpi_object **out)
> +{
> +   union acpi_object *obj;
> +   int rc;
> +
> +   obj = acpi_evaluate_dsm(adev->handle, _acpi_dsm_guid,
> +   WBRF_REVISION, fn, argv4);
> +   if (!obj)
> +   return -ENXIO;
> +
> +   switch (obj->type) {
> +   case ACPI_TYPE_BUFFER:
> +   *out = obj;
> +   return 0;
> +
> +   case ACPI_TYPE_INTEGER:
> +   rc =  obj->integer.value ? -EINVAL : 0;
> +   break;
> +
> +   default:
> +   rc = -EOPNOTSUPP;
> +   }
> +
> +   ACPI_FREE(obj);
> +
> 

Re: [V9 1/9] drivers core: Add support for Wifi band RF mitigations

[Rafael J. Wysocki]

On Fri, Aug 18, 2023 at 5:27 AM Evan Quan  wrote:
>
> Due to electrical and mechanical constraints in certain platform designs
> there may be likely interference of relatively high-powered harmonics of
> the (G-)DDR memory clocks with local radio module frequency bands used
> by Wifi 6/6e/7.
>
> To mitigate this, AMD has introduced a mechanism that devices can use to
> notify active use of particular frequencies so that other devices can make
> relative internal adjustments as necessary to avoid this resonance.
>
> In order for a device to support this, the expected flow for device
> driver or subsystems:
>
> Drivers/subsystems contributing frequencies:
>
> 1) During probe, check `wbrf_supported_producer` to see if WBRF supported
>for the device.
> 2) If adding frequencies, then call `wbrf_add_exclusion` with the
>start and end ranges of the frequencies.
> 3) If removing frequencies, then call `wbrf_remove_exclusion` with
>start and end ranges of the frequencies.
>
> Drivers/subsystems responding to frequencies:
>
> 1) During probe, check `wbrf_supported_consumer` to see if WBRF is supported
>for the device.
> 2) Call the `wbrf_register_notifier` to register for notifications of
>frequency changes from other devices.
> 3) Call the `wbrf_retrieve_exclusions` to retrieve the current exclusions
>range on receiving a notification and response correspondingly.
>
> Meanwhile a kernel parameter `wbrf` with default setting as "auto" is
> introduced to specify what the policy is.
>   - With `wbrf=on`, the WBRF features will be enabled forcely.
>   - With `wbrf=off`, the WBRF features will be disabled forcely.
>   - With `wbrf=auto`, it will be up to the system to do proper checks
> to determine the WBRF features should be enabled or not.
>
> Co-developed-by: Mario Limonciello 
> Signed-off-by: Mario Limonciello 
> Co-developed-by: Evan Quan 
> Signed-off-by: Evan Quan 

In the first place, this requires quite a bit of driver API
documentation that is missing.

To a minimum, it should explain what the interface is for and how it
is supposed to be used by drivers (both "producers" and "consumers").

And how to determine whether or not a given device is a "producer" or
"consumer" from the WBRF perspective.

> --
> v4->v5:
>   - promote this to be a more generic solution with input argument taking
> `struct device` and provide better scalability to support non-ACPI
> scenarios(Andrew)
>   - update the APIs naming and some other minor fixes(Rafael)
> v6->v7:
>   - revised the `struct wbrf_ranges_out` to be naturally aligned(Andrew)
>   - revised some code comments(Andrew)
> v8->v9:
>   - update the document to be more readable(Randy)
> ---
>  .../admin-guide/kernel-parameters.txt |   8 +
>  drivers/base/Makefile |   1 +
>  drivers/base/wbrf.c   | 280 ++
>  include/linux/wbrf.h  |  47 +++
>  4 files changed, 336 insertions(+)
>  create mode 100644 drivers/base/wbrf.c
>  create mode 100644 include/linux/wbrf.h
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> b/Documentation/admin-guide/kernel-parameters.txt
> index a1457995fd41..5566fefeffdf 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -7152,3 +7152,11 @@
> xmon commands.
> off xmon is disabled.
>
> +   wbrf=   [KNL]
> +   Format: { on | auto (default) | off }
> +   Controls if WBRF features should be forced on or off.
> +   on  Force enable the WBRF features.
> +   autoUp to the system to do proper checks to
> +   determine the WBRF features should be enabled
> +   or not.
> +   off Force disable the WBRF features.

Well, how's a casual reader of this file supposed to find out what
WBRF is and whether or not they should care?

> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 3079bfe53d04..7b3cef898c19 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_GENERIC_MSI_IRQ) += platform-msi.o
>  obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o
>  obj-$(CONFIG_GENERIC_ARCH_NUMA) += arch_numa.o
>  obj-$(CONFIG_ACPI) += physical_location.o
> +obj-y  += wbrf.o
>
>  obj-y  += test/
>
> diff --git a/drivers/base/wbrf.c b/drivers/base/wbrf.c
> new file mode 100644
> index ..678f245c12c6
> --- /dev/null
> +++ b/drivers/base/wbrf.c
> @@ -0,0 +1,280 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Wifi Band Exclusion Interface
> + * Copyright (C) 2023 Advanced Micro Devices
> + *

I would expect some explanation of the interface design and purpose here.

So I don't have to wonder what WBRF_POLICY_MODE is 

Re: [PATCH V4 1/8] drivers/acpi: Add support for Wifi band RF mitigations

[Rafael J. Wysocki]

On Fri, Jun 23, 2023 at 6:48 PM Limonciello, Mario
 wrote:
>
>
> On 6/23/2023 11:28 AM, Rafael J. Wysocki wrote:
> > On Fri, Jun 23, 2023 at 5:57 PM Limonciello, Mario
> >  wrote:
> >>
> >> On 6/23/2023 9:52 AM, Rafael J. Wysocki wrote:
> >>> On Wed, Jun 21, 2023 at 7:47 AM Evan Quan  wrote:
> >>>> From: Mario Limonciello 
> >>>>
> >>>> Due to electrical and mechanical constraints in certain platform designs
> >>>> there may be likely interference of relatively high-powered harmonics of
> >>>> the (G-)DDR memory clocks with local radio module frequency bands used
> >>>> by Wifi 6/6e/7.
> >>>>
> >>>> To mitigate this, AMD has introduced an ACPI based mechanism that
> >>>> devices can use to notify active use of particular frequencies so
> >>>> that devices can make relative internal adjustments as necessary
> >>>> to avoid this resonance.
> >>>>
> >>>> In order for a device to support this, the expected flow for device
> >>>> driver or subsystems:
> >>>>
> >>>> Drivers/subsystems contributing frequencies:
> >>>>
> >>>> 1) During probe, check `wbrf_supported_producer` to see if WBRF supported
> >>> The prefix should be acpi_wbrf_ or acpi_amd_wbrf_ even, so it is clear
> >>> that this uses ACPI and is AMD-specific.
> >> I guess if we end up with an intermediary library approach
> >> wbrf_supported_producer makes sense and that could call acpi_wbrf_*.
> >>
> >> But with no intermediate library your suggestion makes sense.
> >>
> >> I would prefer not to make it acpi_amd as there is no reason that
> >> this exact same problem couldn't happen on an
> >> Wifi 6e + Intel SOC + AMD dGPU design too and OEMs could use the
> >> same mitigation mechanism as Wifi6e + AMD SOC + AMD dGPU too.
> > The mitigation mechanism might be the same, but the AML interface very
> > well may be different.
>
>
> Right.  I suppose right now we should keep it prefixed as "amd",
> and if it later is promoted as a standard it can be renamed.
>
>
> >
> > My point is that this particular interface is AMD-specific ATM and I'm
> > not aware of any plans to make it "standard" in some way.
>
>
> Yeah; this implementation is currently AMD specific AML, but I
> expect the exact same AML would be delivered to OEMs using the
> dGPUs.
>
>
> >
> > Also if the given interface is specified somewhere, it would be good
> > to have a pointer to that place.
>
>
> It's a code first implementation.  I'm discussing with the
> owners when they will release it.
>
>
> >
> >>> Whether or not there needs to be an intermediate library wrapped
> >>> around this is a different matter.
> > IMO individual drivers should not be expected to use this interface
> > directly, as that would add to boilerplate code and overall bloat.
>
> The thing is the ACPI method is not a platform method.  It's
> a function of the device (_DSM).

_DSM is an interface to the platform like any other AML, so I'm not
really sure what you mean.

> The reason for having acpi_wbrf.c in the first place is to
> avoid the boilerplate of the _DSM implementation across multiple
> drivers.

Absolutely, drivers should not be bothered with having to use _DSM in
any case.  However, they may not even realize that they are running on
a system using ACPI and I'm not sure if they really should care.

> >
> > Also whoever uses it, would first need to check if the device in
> > question has an ACPI companion.
>
>
> Which comes back to Andrew's point.
> Either we:
>
> Have a generic wbrf_ helper that takes struct *device and
> internally checks if there is an ACPI companion and support.
>
> or
>
> Do the check for support in mac80211 + applicable drivers
> and only call the AMD WBRF ACPI method in those drivers in
> those cases.

Either of the above has problems IMO.

The problem with the wbrf_ helper approach is that it adds
(potentially) several pieces of interaction with the platform,
potentially for every driver, in places where drivers don't do such
things as a rule.

The problem with the other approach is that the drivers in question
now need to be aware of ACPI in general and the AMD WBRF interface in
particular and if other similar interfaces are added by other vendors,
they will have to learn about those as well.

I think that we need to start over with a general problem statement
that in some cases the platform needs to be consulted regarding radio
frequencies that drivers would like to use, because it may need to
adjust or simply say which ranges are "noisy" (or even completely
unusable for that matter).  That should allow us to figure out how the
interface should look like from the driver side and it should be
possible to hook up the existing platform interface to that.

Re: [PATCH V4 1/8] drivers/acpi: Add support for Wifi band RF mitigations

[Rafael J. Wysocki]

On Wed, Jun 21, 2023 at 7:47 AM Evan Quan  wrote:
>
> From: Mario Limonciello 
>
> Due to electrical and mechanical constraints in certain platform designs
> there may be likely interference of relatively high-powered harmonics of
> the (G-)DDR memory clocks with local radio module frequency bands used
> by Wifi 6/6e/7.
>
> To mitigate this, AMD has introduced an ACPI based mechanism that
> devices can use to notify active use of particular frequencies so
> that devices can make relative internal adjustments as necessary
> to avoid this resonance.
>
> In order for a device to support this, the expected flow for device
> driver or subsystems:
>
> Drivers/subsystems contributing frequencies:
>
> 1) During probe, check `wbrf_supported_producer` to see if WBRF supported
>for the device.
> 2) If adding frequencies, then call `wbrf_add_exclusion` with the
>start and end ranges of the frequencies.
> 3) If removing frequencies, then call `wbrf_remove_exclusion` with
>start and end ranges of the frequencies.
>
> Drivers/subsystems responding to frequencies:
>
> 1) During probe, check `wbrf_supported_consumer` to see if WBRF is supported
>for the device.
> 2) Call the `wbrf_retrieve_exclusions` to retrieve the current
>exclusions on receiving an ACPI notification for a new frequency
>change.
>
> Signed-off-by: Mario Limonciello 
> Co-developed-by: Evan Quan 
> Signed-off-by: Evan Quan 
> --
> v1->v2:
>   - move those wlan specific implementations to net/mac80211(Mario)
> ---
>  drivers/acpi/Kconfig |   7 ++
>  drivers/acpi/Makefile|   2 +
>  drivers/acpi/acpi_wbrf.c | 215 +++
>  include/linux/wbrf.h |  55 ++
>  4 files changed, 279 insertions(+)
>  create mode 100644 drivers/acpi/acpi_wbrf.c
>  create mode 100644 include/linux/wbrf.h
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index ccbeab9500ec..0276c1487fa2 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -611,3 +611,10 @@ config X86_PM_TIMER
>
>   You should nearly always say Y here because many modern
>   systems require this timer.
> +
> +config ACPI_WBRF
> +   bool "ACPI Wifi band RF mitigation mechanism"
> +   help
> + Wifi band RF mitigation mechanism allows multiple drivers from
> + different domains to notify the frequencies in use so that hardware
> + can be reconfigured to avoid harmonic conflicts.
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index feb36c0b9446..14863b0c619f 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -131,3 +131,5 @@ obj-y   += dptf/
>  obj-$(CONFIG_ARM64)+= arm64/
>
>  obj-$(CONFIG_ACPI_VIOT)+= viot.o
> +
> +obj-$(CONFIG_ACPI_WBRF)+= acpi_wbrf.o
> diff --git a/drivers/acpi/acpi_wbrf.c b/drivers/acpi/acpi_wbrf.c
> new file mode 100644
> index ..8c275998ac29
> --- /dev/null
> +++ b/drivers/acpi/acpi_wbrf.c
> @@ -0,0 +1,215 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD Wifi Band Exclusion Interface

Where is the AML interface for this defined and how does it work?

> + * Copyright (C) 2023 Advanced Micro Devices
> + *
> + */
> +
> +#include 
> +
> +/* functions */
> +#define WBRF_RECORD0x1
> +#define WBRF_RETRIEVE  0x2
> +
> +/* record actions */
> +#define WBRF_RECORD_ADD0x0
> +#define WBRF_RECORD_REMOVE 0x1
> +
> +#define WBRF_REVISION  0x1
> +
> +static const guid_t wifi_acpi_dsm_guid =
> +   GUID_INIT(0x7b7656cf, 0xdc3d, 0x4c1c,
> + 0x83, 0xe9, 0x66, 0xe7, 0x21, 0xde, 0x30, 0x70);
> +
> +static int wbrf_dsm(struct acpi_device *adev, u8 fn,
> +   union acpi_object *argv4,
> +   union acpi_object **out)
> +{
> +   union acpi_object *obj;
> +   int rc;
> +
> +   obj = acpi_evaluate_dsm(adev->handle, _acpi_dsm_guid,
> +   WBRF_REVISION, fn, argv4);
> +   if (!obj)
> +   return -ENXIO;
> +
> +   switch (obj->type) {
> +   case ACPI_TYPE_BUFFER:
> +   if (!*out) {
> +   rc = -EINVAL;
> +   break;

I'm not sure why you want to return an error in this case.  Did you
really mean !out?

> +   }
> +   *out = obj;
> +   return 0;
> +
> +   case ACPI_TYPE_INTEGER:
> +   rc =  obj->integer.value ? -EINVAL : 0;
> +   break;

An empty line here, please, as you added one after the return statement above.

> +   default:
> +   rc = -EOPNOTSUPP;
> +   }
> +   ACPI_FREE(obj);
> +
> +   return rc;

How does the caller know whether or not they need to free the out
object after calling this function?

> +}
> +
> +static int wbrf_record(struct acpi_device *adev, uint8_t action,
> +  struct wbrf_ranges_in *in)
> +{
> +   union 

Re: [PATCH V4 1/8] drivers/acpi: Add support for Wifi band RF mitigations

[Rafael J. Wysocki]

On Fri, Jun 23, 2023 at 5:57 PM Limonciello, Mario
 wrote:
>
>
> On 6/23/2023 9:52 AM, Rafael J. Wysocki wrote:
> > On Wed, Jun 21, 2023 at 7:47 AM Evan Quan  wrote:
> >> From: Mario Limonciello 
> >>
> >> Due to electrical and mechanical constraints in certain platform designs
> >> there may be likely interference of relatively high-powered harmonics of
> >> the (G-)DDR memory clocks with local radio module frequency bands used
> >> by Wifi 6/6e/7.
> >>
> >> To mitigate this, AMD has introduced an ACPI based mechanism that
> >> devices can use to notify active use of particular frequencies so
> >> that devices can make relative internal adjustments as necessary
> >> to avoid this resonance.
> >>
> >> In order for a device to support this, the expected flow for device
> >> driver or subsystems:
> >>
> >> Drivers/subsystems contributing frequencies:
> >>
> >> 1) During probe, check `wbrf_supported_producer` to see if WBRF supported
> > The prefix should be acpi_wbrf_ or acpi_amd_wbrf_ even, so it is clear
> > that this uses ACPI and is AMD-specific.
>
> I guess if we end up with an intermediary library approach
> wbrf_supported_producer makes sense and that could call acpi_wbrf_*.
>
> But with no intermediate library your suggestion makes sense.
>
> I would prefer not to make it acpi_amd as there is no reason that
> this exact same problem couldn't happen on an
> Wifi 6e + Intel SOC + AMD dGPU design too and OEMs could use the
> same mitigation mechanism as Wifi6e + AMD SOC + AMD dGPU too.

The mitigation mechanism might be the same, but the AML interface very
well may be different.

My point is that this particular interface is AMD-specific ATM and I'm
not aware of any plans to make it "standard" in some way.

Also if the given interface is specified somewhere, it would be good
to have a pointer to that place.

> >
> > Whether or not there needs to be an intermediate library wrapped
> > around this is a different matter.

IMO individual drivers should not be expected to use this interface
directly, as that would add to boilerplate code and overall bloat.

Also whoever uses it, would first need to check if the device in
question has an ACPI companion.

Re: [PATCH V4 1/8] drivers/acpi: Add support for Wifi band RF mitigations

[Rafael J. Wysocki]

On Wed, Jun 21, 2023 at 7:47 AM Evan Quan  wrote:
>
> From: Mario Limonciello 
>
> Due to electrical and mechanical constraints in certain platform designs
> there may be likely interference of relatively high-powered harmonics of
> the (G-)DDR memory clocks with local radio module frequency bands used
> by Wifi 6/6e/7.
>
> To mitigate this, AMD has introduced an ACPI based mechanism that
> devices can use to notify active use of particular frequencies so
> that devices can make relative internal adjustments as necessary
> to avoid this resonance.
>
> In order for a device to support this, the expected flow for device
> driver or subsystems:
>
> Drivers/subsystems contributing frequencies:
>
> 1) During probe, check `wbrf_supported_producer` to see if WBRF supported

The prefix should be acpi_wbrf_ or acpi_amd_wbrf_ even, so it is clear
that this uses ACPI and is AMD-specific.

Whether or not there needs to be an intermediate library wrapped
around this is a different matter.

Re: [PATCH v3 0/3] Adjust ACPI video detection fallback path

[Rafael J. Wysocki]

On Thu, Dec 15, 2022 at 8:38 PM Rafael J. Wysocki  wrote:
>
> On Thu, Dec 15, 2022 at 8:20 PM Limonciello, Mario
>  wrote:
> >
> > [Public]
> >
> > > -Original Message-
> > > From: Limonciello, Mario 
> > > Sent: Thursday, December 8, 2022 10:42
> > > To: Rafael J . Wysocki ; Deucher, Alexander
> > > ; Hans de Goede
> > > 
> > > Cc: amd-gfx@lists.freedesktop.org; linux-a...@vger.kernel.org; Daniel
> > > Dadap ; Limonciello, Mario
> > > 
> > > Subject: [PATCH v3 0/3] Adjust ACPI video detection fallback path
> > >
> > > In kernel 6.1 the backlight registration code was overhauled so that
> > > at most one backlight device got registered. As part of this change
> > > there was code added to still allow making an acpi_video0 device if the
> > > BIOS contained backlight control methods but no native or vendor drivers
> > > registered.
> > >
> > > Even after the overhaul this fallback logic is failing on the BIOS from
> > > a number of motherboard manufacturers supporting Ryzen APUs.
> > > What happens is the amdgpu driver finishes registration and as expected
> > > doesn't create a backlight control device since no eDP panels are 
> > > connected
> > > to a desktop.
> > >
> > > Then 8 seconds later the ACPI video detection code creates an
> > > acpi_video0 device that is non-operational. GNOME then creates a
> > > backlight slider.
> > >
> > > To avoid this situation from happening make two sets of changes:
> > >
> > > Prevent desktop problems w/ fallback logic
> > > --
> > > 1) Add support for the video detect code to let native drivers cancel the
> > > fallback logic if they didn't find a panel.
> > >
> > > This is done this way so that if another driver decides that the ACPI
> > > mechanism is still needed it can instead directly call the registration
> > > function.
> > >
> > > 2) Add code to amdgpu to notify the ACPI video detection code that no 
> > > panel
> > > was detected on an APU.
> > >
> > > Disable fallback logic by default
> > > -
> > > This fallback logic was introduced to prevent regressions in the backlight
> > > overhaul.  As it has been deemed unnecessary by Hans explicitly disable 
> > > the
> > > timeout.  If this turns out to be mistake and this part is reverted, the
> > > other patches for preventing desktop problems will avoid regressions on
> > > desktops.
> > >
> > > Mario Limonciello (3):
> > >   ACPI: video: Allow GPU drivers to report no panels
> > >   drm/amd/display: Report to ACPI video if no panels were found
> > >   ACPI: video: Don't enable fallback path for creating ACPI backlight by
> > > default
> > >
> > >  drivers/acpi/acpi_video.c   | 17 -
> > >  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  4 
> > >  include/acpi/video.h|  2 ++
> > >  3 files changed, 18 insertions(+), 5 deletions(-)
> > >
> > > --
> > > 2.34.1
> >
> > FYI, besides me, this series also tested successfully by one of the
> > reporters to the Red Hat bugzilla.
> >
> > https://bugzilla.redhat.com/show_bug.cgi?id=1783786#c8
>
> Thanks for letting me know!
>
> I'll queue it up for 6.2-rc next week.

Done now, thanks!

Re: [PATCH v3 0/3] Adjust ACPI video detection fallback path

[Rafael J. Wysocki]

On Thu, Dec 15, 2022 at 8:20 PM Limonciello, Mario
 wrote:
>
> [Public]
>
> > -Original Message-
> > From: Limonciello, Mario 
> > Sent: Thursday, December 8, 2022 10:42
> > To: Rafael J . Wysocki ; Deucher, Alexander
> > ; Hans de Goede
> > 
> > Cc: amd-gfx@lists.freedesktop.org; linux-a...@vger.kernel.org; Daniel
> > Dadap ; Limonciello, Mario
> > 
> > Subject: [PATCH v3 0/3] Adjust ACPI video detection fallback path
> >
> > In kernel 6.1 the backlight registration code was overhauled so that
> > at most one backlight device got registered. As part of this change
> > there was code added to still allow making an acpi_video0 device if the
> > BIOS contained backlight control methods but no native or vendor drivers
> > registered.
> >
> > Even after the overhaul this fallback logic is failing on the BIOS from
> > a number of motherboard manufacturers supporting Ryzen APUs.
> > What happens is the amdgpu driver finishes registration and as expected
> > doesn't create a backlight control device since no eDP panels are connected
> > to a desktop.
> >
> > Then 8 seconds later the ACPI video detection code creates an
> > acpi_video0 device that is non-operational. GNOME then creates a
> > backlight slider.
> >
> > To avoid this situation from happening make two sets of changes:
> >
> > Prevent desktop problems w/ fallback logic
> > --
> > 1) Add support for the video detect code to let native drivers cancel the
> > fallback logic if they didn't find a panel.
> >
> > This is done this way so that if another driver decides that the ACPI
> > mechanism is still needed it can instead directly call the registration
> > function.
> >
> > 2) Add code to amdgpu to notify the ACPI video detection code that no panel
> > was detected on an APU.
> >
> > Disable fallback logic by default
> > -
> > This fallback logic was introduced to prevent regressions in the backlight
> > overhaul.  As it has been deemed unnecessary by Hans explicitly disable the
> > timeout.  If this turns out to be mistake and this part is reverted, the
> > other patches for preventing desktop problems will avoid regressions on
> > desktops.
> >
> > Mario Limonciello (3):
> >   ACPI: video: Allow GPU drivers to report no panels
> >   drm/amd/display: Report to ACPI video if no panels were found
> >   ACPI: video: Don't enable fallback path for creating ACPI backlight by
> > default
> >
> >  drivers/acpi/acpi_video.c   | 17 -
> >  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   |  4 
> >  include/acpi/video.h|  2 ++
> >  3 files changed, 18 insertions(+), 5 deletions(-)
> >
> > --
> > 2.34.1
>
> FYI, besides me, this series also tested successfully by one of the
> reporters to the Red Hat bugzilla.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1783786#c8

Thanks for letting me know!

I'll queue it up for 6.2-rc next week.

Re: [PATCH v2 1/3] ACPI: video: Allow GPU drivers to report no panels

[Rafael J. Wysocki]

On Thu, Dec 8, 2022 at 2:09 AM Mario Limonciello
 wrote:
>
> The current logic for the ACPI backlight detection will create
> a backlight device if no native or vendor drivers have created
> 8 seconds after the system has booted if the ACPI tables
> included backlight control methods.
>
> If the GPU drivers have loaded, they may be able to report whether
> any LCD panels were found.  Allow using this information to factor
> in whether to enable the fallback logic for making an acpi_video0
> backlight device.
>
> Suggested-by: Hans de Goede 
> Signed-off-by: Mario Limonciello 
> ---
> v1->v2:
>  * Cancel registration for backlight device instead (Hans)
>  * drop desktop check (Dan)
>
>  drivers/acpi/acpi_video.c | 11 +++
>  include/acpi/video.h  |  1 +
>  2 files changed, 12 insertions(+)
>
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index 32953646caeb..f64fdb029090 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -2178,6 +2178,17 @@ static bool should_check_lcd_flag(void)
> return false;
>  }
>
> +/*
> + * At least one graphics driver has reported that no LCD is connected
> + * via the native interface. cancel the registration for fallback 
> acpi_video0.
> + * If another driver still deems this necessary, it can explicitly register 
> it.
> + */
> +void acpi_video_report_nolcd(void)
> +{
> +   cancel_delayed_work(_bus_register_backlight_work);
> +}
> +EXPORT_SYMBOL(acpi_video_report_nolcd);
> +
>  int acpi_video_register(void)
>  {
> int ret = 0;
> diff --git a/include/acpi/video.h b/include/acpi/video.h
> index a275c35e5249..1fccb111c197 100644
> --- a/include/acpi/video.h
> +++ b/include/acpi/video.h
> @@ -53,6 +53,7 @@ enum acpi_backlight_type {
>  };
>
>  #if IS_ENABLED(CONFIG_ACPI_VIDEO)
> +extern void acpi_video_report_nolcd(void);

It looks like a stub is needed for the other case.  Apparently, things
fail to compile due to the lack of it.

>  extern int acpi_video_register(void);
>  extern void acpi_video_unregister(void);
>  extern void acpi_video_register_backlight(void);
> --
> 2.34.1
>

Re: [PATCH] ACPICA: Fix return

[Rafael J. Wysocki]

On Tue, Nov 8, 2022 at 12:48 PM  wrote:
>
> return is not a function, parentheses are not required
>
> Signed-off-by: KaiLong Wang 

ACPICA material is to be submitted to the upstream project at GitHub
(please see MAINTAINERS for the link).

You may notice, however, that your changes do not align with the
coding style there.

Moreover, the patch contains non-ACPICA changes that are not mentioned
in the changelog.

> ---
>  drivers/acpi/acpica/evsci.c | 12 +---
>  drivers/gpu/drm/amd/display/dc/core/dc_stream.c | 17 +++--
>  2 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/acpi/acpica/evsci.c b/drivers/acpi/acpica/evsci.c
> index 3915ff61412b..63dd2aa2d16a 100644
> --- a/drivers/acpi/acpica/evsci.c
> +++ b/drivers/acpi/acpica/evsci.c
> @@ -38,9 +38,8 @@ u32 acpi_ev_sci_dispatch(void)
>
> /* Are there any host-installed SCI handlers? */
>
> -   if (!acpi_gbl_sci_handler_list) {
> -   return (int_status);
> -   }
> +   if (!acpi_gbl_sci_handler_list)
> +   return int_status;
>
> flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
>
> @@ -57,7 +56,7 @@ u32 acpi_ev_sci_dispatch(void)
> }
>
> acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
> -   return (int_status);
> +   return int_status;
>  }
>
>  
> /***
> @@ -193,9 +192,8 @@ acpi_status acpi_ev_remove_all_sci_handlers(void)
> acpi_os_remove_interrupt_handler((u32) 
> acpi_gbl_FADT.sci_interrupt,
>  acpi_ev_sci_xrupt_handler);
>
> -   if (!acpi_gbl_sci_handler_list) {
> -   return (status);
> -   }
> +   if (!acpi_gbl_sci_handler_list)
> +   return status;
>
> flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
> index 38d71b5c1f2d..1a20117b 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
> @@ -29,7 +29,6 @@
>  #include "core_types.h"
>  #include "resource.h"
>  #include "ipp.h"
> -#include "timing_generator.h"
>  #include "dc_dmub_srv.h"
>
>  #define DC_LOGGER dc->ctx->logger
> @@ -152,9 +151,8 @@ static void dc_stream_free(struct kref *kref)
>
>  void dc_stream_release(struct dc_stream_state *stream)
>  {
> -   if (stream != NULL) {
> +   if (stream != NULL)
> kref_put(>refcount, dc_stream_free);
> -   }
>  }
>
>  struct dc_stream_state *dc_create_stream_for_sink(
> @@ -316,11 +314,11 @@ bool dc_stream_set_cursor_attributes(
> struct dc  *dc;
> bool reset_idle_optimizations = false;
>
> -   if (NULL == stream) {
> +   if (stream == NULL) {
> dm_error("DC: dc_stream is NULL!\n");
> return false;
> }
> -   if (NULL == attributes) {
> +   if (attributes == NULL) {
> dm_error("DC: attributes is NULL!\n");
> return false;
> }
> @@ -399,12 +397,12 @@ bool dc_stream_set_cursor_position(
> struct dc  *dc = stream->ctx->dc;
> bool reset_idle_optimizations = false;
>
> -   if (NULL == stream) {
> +   if (stream == NULL) {
> dm_error("DC: dc_stream is NULL!\n");
> return false;
> }
>
> -   if (NULL == position) {
> +   if (position == NULL) {
> dm_error("DC: cursor position is NULL!\n");
> return false;
> }
> @@ -468,9 +466,8 @@ bool dc_stream_add_writeback(struct dc *dc,
> }
> }
>
> -   if (!isDrc) {
> +   if (!isDrc)
> stream->writeback_info[stream->num_wb_info++] = *wb_info;
> -   }
>
> if (dc->hwss.enable_writeback) {
> struct dc_stream_status *stream_status = 
> dc_stream_get_status(stream);
> @@ -526,7 +523,7 @@ bool dc_stream_remove_writeback(struct dc *dc,
> /* remove writeback info for disabled writeback pipes from stream */
> for (i = 0, j = 0; i < stream->num_wb_info; i++) {
> if (stream->writeback_info[i].wb_enabled) {
> -   if (j < i)
> +   if (i != j)
> /* trim the array */
> stream->writeback_info[j] = 
> stream->writeback_info[i];
> j++;
> --
> 2.36.1

Re: [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)

[Rafael J. Wysocki]

On Thu, Oct 27, 2022 at 2:17 PM Hans de Goede  wrote:
>
> Hi,
>
> On 10/27/22 14:09, Rafael J. Wysocki wrote:
> > On Thu, Oct 27, 2022 at 12:37 PM Hans de Goede  wrote:
> >>
> >> Hi,
> >>
> >> On 10/27/22 11:52, Matthew Garrett wrote:
> >>> On Thu, Oct 27, 2022 at 11:39:38AM +0200, Hans de Goede wrote:
> >>>
> >>>> The *only* behavior which actually is new in 6.1 is the native GPU
> >>>> drivers now doing the equivalent of:
> >>>>
> >>>>  if (acpi_video_get_backlight_type() != acpi_backlight_native)
> >>>>  return;
> >>>>
> >>>> In their backlight register paths (i), which is causing the native
> >>>> backlight to disappear on your custom laptop setup and on Chromebooks
> >>>> (with the Chromebooks case being already solved I hope.).
> >>>
> >>> It's causing the backlight control to vanish on any machine that isn't
> >>> ((acpi_video || vendor interface) || !acpi). Most machines that fall
> >>> into that are either weird or Chromebooks or old, but there are machines
> >>> that fall into that.
> >>
> >> I acknowledge that their are machines that fall into this category,
> >> but I expect / hope there to be so few of them that we can just DMI
> >> quirk our way out if this.
> >>
> >> I believe the old group to be small because:
> >>
> >> 1. Generally speaking the "native" control method is usually not
> >> present on the really old (pre ACPI video spec) mobile GPUs.
> >>
> >> 2. On most old laptops I would still expect there to be a vendor
> >> interface too, and if both get registered standard desktop environments
> >> will prefer the vendor one, so then we need a native DMI quirk to
> >> disable the vendor interface anyways and we already have a bunch of
> >> those, so some laptops in this group are already covered by DMI quirks.
> >>
> >> And a fix for the Chromebook case is already in Linus' tree, which
> >> just leaves the weird case, of which there will hopefully be only
> >> a few.
> >>
> >> I do share your worry that this might break some machines, but
> >> the only way to really find out is to get this code out there
> >> I'm afraid.
> >>
> >> I have just written a blog post asking for people to check if
> >> their laptop might be affected; and to report various details
> >> to me of their laptop is affected:
> >>
> >> https://hansdegoede.dreamwidth.org/26548.html
> >>
> >> Lets wait and see how this goes. If I get (too) many reports then
> >> I will send a revert of the addition of the:
> >>
> >> if (acpi_video_get_backlight_type() != acpi_backlight_native)
> >> return;
> >>
> >> check to the i915 / radeon / amd / nouveau drivers.
> >>
> >> (And if I only get a couple of reports I will probably just submit
> >> DMI quirks for the affected models).
> >
> > Sounds reasonable to me, FWIW.
> >
> > And IIUC the check above can be overridden by passing
> > acpi_backlight=native in the kernel command line, right?
>
> Right, that can be used as a quick workaround, but we really do
> want this to work OOTB everywhere.

Sure.

My point is that if it doesn't work OOTB for someone, and say it used
to, they can use the above workaround and report back.

Re: [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)

[Rafael J. Wysocki]

On Thu, Oct 27, 2022 at 12:37 PM Hans de Goede  wrote:
>
> Hi,
>
> On 10/27/22 11:52, Matthew Garrett wrote:
> > On Thu, Oct 27, 2022 at 11:39:38AM +0200, Hans de Goede wrote:
> >
> >> The *only* behavior which actually is new in 6.1 is the native GPU
> >> drivers now doing the equivalent of:
> >>
> >>  if (acpi_video_get_backlight_type() != acpi_backlight_native)
> >>  return;
> >>
> >> In their backlight register paths (i), which is causing the native
> >> backlight to disappear on your custom laptop setup and on Chromebooks
> >> (with the Chromebooks case being already solved I hope.).
> >
> > It's causing the backlight control to vanish on any machine that isn't
> > ((acpi_video || vendor interface) || !acpi). Most machines that fall
> > into that are either weird or Chromebooks or old, but there are machines
> > that fall into that.
>
> I acknowledge that their are machines that fall into this category,
> but I expect / hope there to be so few of them that we can just DMI
> quirk our way out if this.
>
> I believe the old group to be small because:
>
> 1. Generally speaking the "native" control method is usually not
> present on the really old (pre ACPI video spec) mobile GPUs.
>
> 2. On most old laptops I would still expect there to be a vendor
> interface too, and if both get registered standard desktop environments
> will prefer the vendor one, so then we need a native DMI quirk to
> disable the vendor interface anyways and we already have a bunch of
> those, so some laptops in this group are already covered by DMI quirks.
>
> And a fix for the Chromebook case is already in Linus' tree, which
> just leaves the weird case, of which there will hopefully be only
> a few.
>
> I do share your worry that this might break some machines, but
> the only way to really find out is to get this code out there
> I'm afraid.
>
> I have just written a blog post asking for people to check if
> their laptop might be affected; and to report various details
> to me of their laptop is affected:
>
> https://hansdegoede.dreamwidth.org/26548.html
>
> Lets wait and see how this goes. If I get (too) many reports then
> I will send a revert of the addition of the:
>
> if (acpi_video_get_backlight_type() != acpi_backlight_native)
> return;
>
> check to the i915 / radeon / amd / nouveau drivers.
>
> (And if I only get a couple of reports I will probably just submit
> DMI quirks for the affected models).

Sounds reasonable to me, FWIW.

And IIUC the check above can be overridden by passing
acpi_backlight=native in the kernel command line, right?

[PATCH] drm: amd: amdgpu: ACPI: Add comment about ACPI_FADT_LOW_POWER_S0

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

According to the ACPI specification [1], the ACPI_FADT_LOW_POWER_S0
flag merely means that it is better to use low-power S0 idle on the
given platform than S3 (provided that the latter is supported) and it
doesn't preclude using either of them (which of them will be used
depends on the choices made by user space).

However, on some systems that flag is used to indicate whether or not
to enable special firmware mechanics allowing the system to save more
energy when suspended to idle.  If that flag is unset, doing so is
generally risky.

Accordingly, add a comment to explain the ACPI_FADT_LOW_POWER_S0 check
in amdgpu_acpi_is_s0ix_active(), the purpose of which is otherwise
somewhat unclear.

Link: 
https://uefi.org/specs/ACPI/6.4/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#fixed-acpi-description-table-fadt
 # [1]
Signed-off-by: Rafael J. Wysocki 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c |6 ++
 1 file changed, 6 insertions(+)

Index: linux-pm/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
===
--- linux-pm.orig/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
+++ linux-pm/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c
@@ -1066,6 +1066,12 @@ bool amdgpu_acpi_is_s0ix_active(struct a
(pm_suspend_target_state != PM_SUSPEND_TO_IDLE))
return false;
 
+   /*
+* If ACPI_FADT_LOW_POWER_S0 is not set in the FADT, it is generally
+* risky to do any special firmware-related preparations for entering
+* S0ix even though the system is suspending to idle, so return false
+* in that case.
+*/
if (!(acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0)) {
dev_warn_once(adev->dev,
  "Power consumption will be higher as BIOS has not 
been configured for suspend-to-idle.\n"

Re: [PATCH v2 00/29] drm/kms: Stop registering multiple /sys/class/backlight devs for a single display

[Rafael J. Wysocki]

On Tue, Jul 12, 2022 at 9:39 PM Hans de Goede  wrote:
>
> Hi All,
>
> As mentioned in my RFC titled "drm/kms: control display brightness through
> drm_connector properties":
> https://lore.kernel.org/dri-devel/0d188965-d809-81b5-74ce-7d30c49fe...@redhat.com/
>
> The first step towards this is to deal with some existing technical debt
> in backlight handling on x86/ACPI boards, specifically we need to stop
> registering multiple /sys/class/backlight devs for a single display.
>
> This series implements my RFC describing my plan for these cleanups:
> https://lore.kernel.org/dri-devel/98519ba0-7f18-201a-ea34-652f50343...@redhat.com/
>
> This new version addresses the few small remarks made on version 1 (mainly
> changing patch 1/29) and more importantly this finishes the refactoring by
> else addressing all the bits from the "Other issues" section of
> the refactor RFC (resulting in patches 15-29 which are new in v2).
>
> Please review and test! I hope to be able to make an immutable branch
> based on 5.20-rc1 + this series available for merging into the various
> touched subsystems once 5.20-rc2 is out.

Please feel free to add

Acked-by: Rafael J. Wysocki 

to all of the ACPI video patches in this series.

Thanks!

> Hans de Goede (29):
>   ACPI: video: Add acpi_video_backlight_use_native() helper
>   drm/i915: Don't register backlight when another backlight should be
> used
>   drm/amdgpu: Don't register backlight when another backlight should be
> used
>   drm/radeon: Don't register backlight when another backlight should be
> used
>   drm/nouveau: Don't register backlight when another backlight should be
> used
>   ACPI: video: Drop backlight_device_get_by_type() call from
> acpi_video_get_backlight_type()
>   ACPI: video: Remove acpi_video_bus from list before tearing it down
>   ACPI: video: Simplify acpi_video_unregister_backlight()
>   ACPI: video: Make backlight class device registration a separate step
>   ACPI: video: Remove code to unregister acpi_video backlight when a
> native backlight registers
>   drm/i915: Call acpi_video_register_backlight() (v2)
>   drm/nouveau: Register ACPI video backlight when nv_backlight
> registration fails
>   drm/amdgpu: Register ACPI video backlight when skipping amdgpu
> backlight registration
>   drm/radeon: Register ACPI video backlight when skipping radeon
> backlight registration
>   ACPI: video: Refactor acpi_video_get_backlight_type() a bit
>   ACPI: video: Add Nvidia WMI EC brightness control detection
>   ACPI: video: Add Apple GMUX brightness control detection
>   platform/x86: apple-gmux: Stop calling acpi/video.h functions
>   platform/x86: toshiba_acpi: Stop using
> acpi_video_set_dmi_backlight_type()
>   platform/x86: acer-wmi: Move backlight DMI quirks to
> acpi/video_detect.c
>   platform/x86: asus-wmi: Drop DMI chassis-type check from backlight
> handling
>   platform/x86: asus-wmi: Move acpi_backlight=vendor quirks to ACPI
> video_detect.c
>   platform/x86: asus-wmi: Move acpi_backlight=native quirks to ACPI
> video_detect.c
>   platform/x86: samsung-laptop: Move acpi_backlight=[vendor|native]
> quirks to ACPI video_detect.c
>   ACPI: video: Remove acpi_video_set_dmi_backlight_type()
>   ACPI: video: Drop "Samsung X360" acpi_backlight=native quirk
>   ACPI: video: Drop Clevo/TUXEDO NL5xRU and NL5xNU acpi_backlight=native
> quirks
>   ACPI: video: Fix indentation of video_detect_dmi_table[] entries
>   drm/todo: Add entry about dealing with brightness control on devices
> with > 1 panel
>
>  Documentation/gpu/todo.rst|  68 +++
>  drivers/acpi/Kconfig  |   1 +
>  drivers/acpi/acpi_video.c |  59 ++-
>  drivers/acpi/video_detect.c   | 415 +++---
>  drivers/gpu/drm/Kconfig   |  12 +
>  .../gpu/drm/amd/amdgpu/atombios_encoders.c|  14 +-
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   9 +
>  drivers/gpu/drm/gma500/Kconfig|   2 +
>  drivers/gpu/drm/i915/Kconfig  |   2 +
>  .../gpu/drm/i915/display/intel_backlight.c|   7 +
>  drivers/gpu/drm/i915/display/intel_display.c  |   8 +
>  drivers/gpu/drm/i915/display/intel_panel.c|   3 +
>  drivers/gpu/drm/i915/i915_drv.h   |   2 +
>  drivers/gpu/drm/nouveau/nouveau_backlight.c   |  14 +
>  drivers/gpu/drm/radeon/atombios_encoders.c|   7 +
>  drivers/gpu/drm/radeon/radeon_encoders.c  |  11 +-
>  .../gpu/drm/radeon/radeon_legacy_encoders.c   |   7 +
>  drivers/platform/x86/acer-wmi.c   |  66 ---
>  drivers/platform/x86/apple-gmux.c |   3 -
>  dr

Re: [PATCH] ACPI: PM: s2idle: Add missing LPS0 functions for AMD

[Rafael J. Wysocki]

On Wed, May 5, 2021 at 3:21 PM Alex Deucher  wrote:
>
> These are supposedly not required for AMD platforms,
> but at least some HP laptops seem to require it to
> properly turn off the keyboard backlight.
>
> Based on a patch from Marcin Bachry .
>
> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1230
> Reviewed-by: Hans de Goede 
> Signed-off-by: Alex Deucher 
> Cc: Marcin Bachry 
> Cc: Mario Limonciello 
> ---
>
> Resend with updated subject.
>
>  drivers/acpi/x86/s2idle.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index 2b69536cdccb..2d7ddb8a8cb6 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -42,6 +42,8 @@ static const struct acpi_device_id lps0_device_ids[] = {
>
>  /* AMD */
>  #define ACPI_LPS0_DSM_UUID_AMD  "e3f32452-febc-43ce-9039-932122d37721"
> +#define ACPI_LPS0_ENTRY_AMD 2
> +#define ACPI_LPS0_EXIT_AMD  3
>  #define ACPI_LPS0_SCREEN_OFF_AMD4
>  #define ACPI_LPS0_SCREEN_ON_AMD 5
>
> @@ -408,6 +410,7 @@ int acpi_s2idle_prepare_late(void)
>
> if (acpi_s2idle_vendor_amd()) {
> acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF_AMD);
> +   acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY_AMD);
> } else {
> acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_OFF);
> acpi_sleep_run_lps0_dsm(ACPI_LPS0_ENTRY);
> @@ -422,6 +425,7 @@ void acpi_s2idle_restore_early(void)
> return;
>
> if (acpi_s2idle_vendor_amd()) {
> +   acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT_AMD);
> acpi_sleep_run_lps0_dsm(ACPI_LPS0_SCREEN_ON_AMD);
> } else {
> acpi_sleep_run_lps0_dsm(ACPI_LPS0_EXIT);
> --

Applied as 5.14 material, thanks!
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] ACPI: Test for ACPI_SUCCESS rather than !ACPI_FAILURE

[Rafael J. Wysocki]

On Wed, Jan 27, 2021 at 5:06 PM Bjorn Helgaas  wrote:
>
> On Wed, Jan 27, 2021 at 04:44:02PM +0100, Rafael J. Wysocki wrote:
> > On Wed, Jan 27, 2021 at 4:16 PM Bjorn Helgaas  wrote:
> > >
> > > On Tue, Jan 26, 2021 at 02:23:17PM -0600, Bjorn Helgaas wrote:
> > > > From: Bjorn Helgaas 
> > > >
> > > > The double negative makes it hard to read "if (!ACPI_FAILURE(status))".
> > > > Replace it with "if (ACPI_SUCCESS(status))".
> > > >
> > > > Signed-off-by: Bjorn Helgaas 
> > > > ---
> > > >
> > > > This isn't really an ACPI patch, but I'm sending it to you, Rafael, 
> > > > since
> > > > it seems easier to just apply these all at once.  But I'd be happy to 
> > > > split
> > > > them up into individual patches if you'd rather.
> > >
> > > Thanks, everybody.  Rafael, I'll just merge this via my tree to avoid
> > > burdening you.
> >
> > It may conflict with some janitorial stuff I'm doing, though, so
> > unless you've already applied it, I'd prefer to take it via the ACPI
> > tree.
>
> No problem, it's all yours!

Applied as 5.12 material with the ACKs, thanks!
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] ACPI: Test for ACPI_SUCCESS rather than !ACPI_FAILURE

[Rafael J. Wysocki]

On Wed, Jan 27, 2021 at 4:16 PM Bjorn Helgaas  wrote:
>
> On Tue, Jan 26, 2021 at 02:23:17PM -0600, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas 
> >
> > The double negative makes it hard to read "if (!ACPI_FAILURE(status))".
> > Replace it with "if (ACPI_SUCCESS(status))".
> >
> > Signed-off-by: Bjorn Helgaas 
> > ---
> >
> > This isn't really an ACPI patch, but I'm sending it to you, Rafael, since
> > it seems easier to just apply these all at once.  But I'd be happy to split
> > them up into individual patches if you'd rather.
>
> Thanks, everybody.  Rafael, I'll just merge this via my tree to avoid
> burdening you.

It may conflict with some janitorial stuff I'm doing, though, so
unless you've already applied it, I'd prefer to take it via the ACPI
tree.

Cheers!
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Rafael J. Wysocki]

On Mon, Nov 23, 2020 at 4:58 PM James Bottomley
 wrote:
>
> On Mon, 2020-11-23 at 15:19 +0100, Miguel Ojeda wrote:
> > On Sun, Nov 22, 2020 at 11:36 PM James Bottomley
> >  wrote:

[cut]

> >
> > Maintainers routinely review 1-line trivial patches, not to mention
> > internal API changes, etc.
>
> We're also complaining about the inability to recruit maintainers:
>
> https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/
>
> And burn out:
>
> http://antirez.com/news/129

Right.

> The whole crux of your argument seems to be maintainers' time isn't
> important so we should accept all trivial patches ... I'm pushing back
> on that assumption in two places, firstly the valulessness of the time
> and secondly that all trivial patches are valuable.
>
> > If some company does not want to pay for that, that's fine, but they
> > don't get to be maintainers and claim `Supported`.
>
> What I'm actually trying to articulate is a way of measuring value of
> the patch vs cost ... it has nothing really to do with who foots the
> actual bill.
>
> One thesis I'm actually starting to formulate is that this continual
> devaluing of maintainers is why we have so much difficulty keeping and
> recruiting them.

Absolutely.

This is just one of the factors involved, but a significant one IMV.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed by ACPI

[Rafael J. Wysocki]

On Fri, Oct 2, 2020 at 4:20 PM Deucher, Alexander
 wrote:
>
> [AMD Public Use]
>
> > -Original Message-
> > From: amd-gfx  On Behalf Of
> > Rafael J. Wysocki
> > Sent: Friday, October 2, 2020 10:17 AM
> > To: Lukas Wunner 
> > Cc: Aaron Zakhrov ; Michal Rostecki
> > ; Linux PCI ; Rafael J.
> > Wysocki ; amd-gfx list  > g...@lists.freedesktop.org>; ACPI Devel Maling List  > a...@vger.kernel.org>; Shai Coleman ; Bjorn
> > Helgaas ; Arthur Borsboom
> > ; matoro ; Deucher,
> > Alexander ; Mika Westerberg
> > ; Len Brown 
> > Subject: Re: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power
> > managed by ACPI
> >
> > On Fri, Oct 2, 2020 at 7:17 AM Lukas Wunner  wrote:
> > >
> > > Recent laptops with dual AMD GPUs fail to suspend the discrete GPU,
> > > thus causing lockups on system sleep and high power consumption at
> > runtime.
> > > The discrete GPU would normally be suspended to D3cold by turning off
> > > ACPI _PR3 Power Resources of the Root Port above the GPU.
> > >
> > > However on affected systems, the Root Port is hotplug-capable and
> > > pci_bridge_d3_possible() only allows hotplug ports to go to D3 if they
> > > belong to a Thunderbolt device or if the Root Port possesses a
> > > "HotPlugSupportInD3" ACPI property.  Neither is the case on affected
> > > laptops.  The reason for whitelisting only specific, known to work
> > > hotplug ports for D3 is that there have been reports of SkyLake
> > > Xeon-SP systems raising Hardware Error NMIs upon suspending their
> > hotplug ports:
> > >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > > .kernel.org%2Flinux-pci%2F20170503180426.GA4058%40otc-nc-
> > 03%2Fdat
> > >
> > a=02%7C01%7Calexander.deucher%40amd.com%7C99ec20b6d4dc410baf800
> > 8d866dd
> > >
> > e688%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6373724505855
> > 84491
> > >
> > mp;sdata=EPFyxPA0MDBuAkvH7bbp2wHYnpos8p%2BoZmzlUvvdAek%3D
> > mp;reserved
> > > =0
> > >
> > > But if a hotplug port is power manageable by ACPI (as can be detected
> > > through presence of Power Resources and corresponding _PS0 and _PS3
> > > methods) then it ought to be safe to suspend it to D3.  To this end,
> > > amend acpi_pci_bridge_d3() to whitelist such ports for D3.
> > >
> > > Link:
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > > ab.freedesktop.org%2Fdrm%2Famd%2F-
> > %2Fissues%2F1222data=02%7C01%7C
> > >
> > alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> > 7C3dd89
> > >
> > 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491sd
> > ata=cMj
> > >
> > LDIbjp8RQiWX8pgK2bDUH%2B0u3oquy3TqeT9QjZGE%3Dreserved=0
> > > Link:
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > > ab.freedesktop.org%2Fdrm%2Famd%2F-
> > %2Fissues%2F1252data=02%7C01%7C
> > >
> > alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> > 7C3dd89
> > >
> > 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491sd
> > ata=iP9
> > >
> > EqNcM15Dj4Ax%2BE6e2HaMWHEX%2B0IO3cMoi0NXWGzM%3Dreser
> > ved=0
> > > Link:
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > > ab.freedesktop.org%2Fdrm%2Famd%2F-
> > %2Fissues%2F1304data=02%7C01%7C
> > >
> > alexander.deucher%40amd.com%7C99ec20b6d4dc410baf8008d866dde688%
> > 7C3dd89
> > >
> > 61fe4884e608e11a82d994e183d%7C0%7C0%7C637372450585584491sd
> > ata=VlT
> > > UV2UCH4RvKgTXZcpGOpkjZpfijmPgwtvKx6HRT04%3Dreserved=0
> > > Reported-and-tested-by: Arthur Borsboom 
> > > Reported-and-tested-by: matoro 
> > > Reported-by: Aaron Zakhrov 
> > > Reported-by: Michal Rostecki 
> > > Reported-by: Shai Coleman 
> > > Signed-off-by: Lukas Wunner 
> > > Cc: sta...@vger.kernel.org
> > > Cc: Alex Deucher 
> > > Cc: Rafael J. Wysocki 
> > > Cc: Mika Westerberg 
> > > ---
> > >  drivers/pci/pci-acpi.c | 10 ++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index
> > > d5869a0..d9aa551 100644
> > > --- a/drivers/pci/pci-acpi.c
> > > +++ b/drivers/pci/pci-acpi.c
> > > @@ -944,6 +944,16 @@ static bool acpi_pci_bridge_d3(struct pci_dev
> >

Re: [PATCH] PCI/ACPI: Whitelist hotplug ports for D3 if power managed by ACPI

[Rafael J. Wysocki]

On Fri, Oct 2, 2020 at 7:17 AM Lukas Wunner  wrote:
>
> Recent laptops with dual AMD GPUs fail to suspend the discrete GPU, thus
> causing lockups on system sleep and high power consumption at runtime.
> The discrete GPU would normally be suspended to D3cold by turning off
> ACPI _PR3 Power Resources of the Root Port above the GPU.
>
> However on affected systems, the Root Port is hotplug-capable and
> pci_bridge_d3_possible() only allows hotplug ports to go to D3 if they
> belong to a Thunderbolt device or if the Root Port possesses a
> "HotPlugSupportInD3" ACPI property.  Neither is the case on affected
> laptops.  The reason for whitelisting only specific, known to work
> hotplug ports for D3 is that there have been reports of SkyLake Xeon-SP
> systems raising Hardware Error NMIs upon suspending their hotplug ports:
> https://lore.kernel.org/linux-pci/20170503180426.GA4058@otc-nc-03/
>
> But if a hotplug port is power manageable by ACPI (as can be detected
> through presence of Power Resources and corresponding _PS0 and _PS3
> methods) then it ought to be safe to suspend it to D3.  To this end,
> amend acpi_pci_bridge_d3() to whitelist such ports for D3.
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1222
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1252
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1304
> Reported-and-tested-by: Arthur Borsboom 
> Reported-and-tested-by: matoro 
> Reported-by: Aaron Zakhrov 
> Reported-by: Michal Rostecki 
> Reported-by: Shai Coleman 
> Signed-off-by: Lukas Wunner 
> Cc: sta...@vger.kernel.org
> Cc: Alex Deucher 
> Cc: Rafael J. Wysocki 
> Cc: Mika Westerberg 
> ---
>  drivers/pci/pci-acpi.c | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index d5869a0..d9aa551 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -944,6 +944,16 @@ static bool acpi_pci_bridge_d3(struct pci_dev *dev)
> if (!dev->is_hotplug_bridge)
> return false;
>
> +   /* Assume D3 support if the bridge is power-manageable by ACPI. */
> +   adev = ACPI_COMPANION(>dev);
> +   if (!adev && !pci_dev_is_added(dev)) {
> +   adev = acpi_pci_find_companion(>dev);
> +   ACPI_COMPANION_SET(>dev, adev);
> +   }
> +
> +   if (adev && acpi_device_power_manageable(adev))
> +   return true;
> +
> /*
>  * Look for a special _DSD property for the root port and if it
>  * is set we know the hierarchy behind it supports D3 just fine.
> --

I'm going to apply this patch for 5.10 unless Bjorn would rather route
it through the PCI tree.

Thanks!
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/5] power: avs: smartreflex: Remove superfluous cast in debugfs_create_file() call

[Rafael J. Wysocki]

On Monday, October 21, 2019 4:51:48 PM CET Geert Uytterhoeven wrote:
> There is no need to cast a typed pointer to a void pointer when calling
> a function that accepts the latter.  Remove it, as the cast prevents
> further compiler checks.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/power/avs/smartreflex.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
> index 4684e7df833a81e9..5376f3d22f31eade 100644
> --- a/drivers/power/avs/smartreflex.c
> +++ b/drivers/power/avs/smartreflex.c
> @@ -905,7 +905,7 @@ static int omap_sr_probe(struct platform_device *pdev)
>   sr_info->dbg_dir = debugfs_create_dir(sr_info->name, sr_dbg_dir);
>  
>   debugfs_create_file("autocomp", S_IRUGO | S_IWUSR, sr_info->dbg_dir,
> - (void *)sr_info, _sr_fops);
> + sr_info, _sr_fops);
>   debugfs_create_x32("errweight", S_IRUGO, sr_info->dbg_dir,
>  _info->err_weight);
>   debugfs_create_x32("errmaxlimit", S_IRUGO, sr_info->dbg_dir,
> 

Applying as 5.5 material, thanks!




___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/5] power: avs: smartreflex: Remove superfluous cast in debugfs_create_file() call

[Rafael J. Wysocki]

On Monday, October 21, 2019 4:51:48 PM CET Geert Uytterhoeven wrote:
> There is no need to cast a typed pointer to a void pointer when calling
> a function that accepts the latter.  Remove it, as the cast prevents
> further compiler checks.
> 
> Signed-off-by: Geert Uytterhoeven 

Greg, have you taken this one by any chance?

> ---
>  drivers/power/avs/smartreflex.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/power/avs/smartreflex.c b/drivers/power/avs/smartreflex.c
> index 4684e7df833a81e9..5376f3d22f31eade 100644
> --- a/drivers/power/avs/smartreflex.c
> +++ b/drivers/power/avs/smartreflex.c
> @@ -905,7 +905,7 @@ static int omap_sr_probe(struct platform_device *pdev)
>   sr_info->dbg_dir = debugfs_create_dir(sr_info->name, sr_dbg_dir);
>  
>   debugfs_create_file("autocomp", S_IRUGO | S_IWUSR, sr_info->dbg_dir,
> - (void *)sr_info, _sr_fops);
> + sr_info, _sr_fops);
>   debugfs_create_x32("errweight", S_IRUGO, sr_info->dbg_dir,
>  _info->err_weight);
>   debugfs_create_x32("errmaxlimit", S_IRUGO, sr_info->dbg_dir,
> 




___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Set DPM_FLAG_NEVER_SKIP when enabling PM-runtime

[Rafael J. Wysocki]

On 2/18/2019 11:19 PM, Alex Deucher wrote:

Based on a similar patch from Rafael for radeon.

When using ATPX to control dGPU power, the state is not retained
across suspend and resume cycles by default.  This can probably
be loosened for Hybrid Graphics (_PR3) laptops where I think the
state is properly retained.

Fixes: c62ec4610c40 ("PM / core: Fix direct_complete handling for devices with no 
callbacks")
Cc: Rafael J. Wysocki 
Signed-off-by: Alex Deucher 


Acked-by: Rafael J. Wysocki 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index d63cb53ff2bb..eaf90cdc848d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -212,6 +212,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned 
long flags)
}
  
  	if (amdgpu_device_is_px(dev)) {

+   dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NEVER_SKIP);
pm_runtime_use_autosuspend(dev->dev);
pm_runtime_set_autosuspend_delay(dev->dev, 5000);
pm_runtime_set_active(dev->dev);



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] gpu: drm: radeon: Set DPM_FLAG_NEVER_SKIP when enabling PM-runtime

[Rafael J. Wysocki]

On Sun, Feb 17, 2019 at 12:37 AM Alex Deucher  wrote:
>
> On Sat, Feb 16, 2019 at 1:01 AM Lukas Wunner  wrote:
> >
> > On Fri, Feb 15, 2019 at 11:01:04AM -0500, Alex Deucher wrote:
> > > On Fri, Feb 15, 2019 at 10:39 AM Rafael J. Wysocki  
> > > wrote:
> > > > On HP ProBook 4540s, if PM-runtime is enabled in the radeon driver
> > > > and the direct-complete optimization is used for the radeon device
> > > > during system-wide suspend, the system doesn't resume.
> > > >
> > > > Preventing direct-complete from being used with the radeon device by
> > > > setting the DPM_FLAG_NEVER_SKIP driver flag for it makes the problem
> > > > go away, which indicates that direct-complete is not safe for the
> > > > radeon driver in general and should not be used with it (at least
> > > > for now).
> > > >
> > > > This fixes a regression introduced by commit c62ec4610c40
> > > > ("PM / core: Fix direct_complete handling for devices with no
> > > > callbacks") which allowed direct-complete to be applied to
> > > > devices without PM callbacks (again) which in turn unlocked
> > > > direct-complete for radeon on HP ProBook 4540s.
> > >
> > > Do other similar drivers like amdgpu and nouveau need the same fix?
> > > I'm not too familiar with the direct_complete feature in general.
> >
> > direct_complete means that a discrete GPU which is in D3cold upon
> > entering system sleep is left as is, i.e. it is not woken.  It is
> > also expected to still be in D3cold when resuming from system sleep
> > from the PM core's point of view.  (If it is in D0uninitialized, the
> > GPU's driver needs to ensure it is transitioned to D3cold again.)
> >
> > I know for a fact that resuming the discrete GPU is not necessary
> > on my MacBook Pro with Nvidia GPU.  I'd expect those with AMD GPUs
> > to behave the same.  The apple-gmux driver takes care of putting
> > the GPU into D3cold on resume from system sleep if it was in D3cold
> > when entering system sleep (see drivers/platform/x86/apple-gmux.c,
> > gmux_resume()).
> >
> > I think it is desirable to use direct_complete because it saves power
> > (no need to gratuitously wake the GPU upon entering system sleep,
> > only to immediately cut its power) and it also speeds up the suspend
> > process by about half a second.
>
> Thanks for the info.  It sounds like we need a similar patch for
> amdgpu.  With dGPUs controlled by the ACPI ATPX method, I believe the
> dGPU is powered by automatically on resume from S3/S4.  I think there
> may be a way to change that behavior in some revisions of ATPX (i.e.,
> to keep the state across suspend cycles), but it's not the default.
> I'm not sure about the newer _PR3 stuff in Hybrid Graphics laptops.  I
> think it retains state.  In both radeon and amdgpu we probably need to
> check if the system is using ATPX or _PR3 and disable direct complete
> for ATPX at least.

I would disable direct-complete entirely for them then and possibly
consider using DPM_FLAG_SMART_SUSPEND in the cases when that would be
safe.

Anyway, I posted this patch for radeon, because it addresses a
specific regression and I'm not super-familiar with GPU drivers in
general.

Cheers,
Rafael
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] gpu: drm: radeon: Set DPM_FLAG_NEVER_SKIP when enabling PM-runtime

[Rafael J. Wysocki]

From: Rafael J. Wysocki 

On HP ProBook 4540s, if PM-runtime is enabled in the radeon driver
and the direct-complete optimization is used for the radeon device
during system-wide suspend, the system doesn't resume.

Preventing direct-complete from being used with the radeon device by
setting the DPM_FLAG_NEVER_SKIP driver flag for it makes the problem
go away, which indicates that direct-complete is not safe for the
radeon driver in general and should not be used with it (at least
for now).

This fixes a regression introduced by commit c62ec4610c40
("PM / core: Fix direct_complete handling for devices with no
callbacks") which allowed direct-complete to be applied to
devices without PM callbacks (again) which in turn unlocked
direct-complete for radeon on HP ProBook 4540s.

Fixes: c62ec4610c40 ("PM / core: Fix direct_complete handling for devices with 
no callbacks")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=201519
Reported-by: Ярослав Семченко 
Tested-by: Ярослав Семченко 
Signed-off-by: Rafael J. Wysocki 
---
 drivers/gpu/drm/radeon/radeon_kms.c |1 +
 1 file changed, 1 insertion(+)

Index: linux-pm/drivers/gpu/drm/radeon/radeon_kms.c
===
--- linux-pm.orig/drivers/gpu/drm/radeon/radeon_kms.c
+++ linux-pm/drivers/gpu/drm/radeon/radeon_kms.c
@@ -172,6 +172,7 @@ int radeon_driver_load_kms(struct drm_de
}
 
if (radeon_is_px(dev)) {
+   dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NEVER_SKIP);
pm_runtime_use_autosuspend(dev->dev);
pm_runtime_set_autosuspend_delay(dev->dev, 5000);
pm_runtime_set_active(dev->dev);

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [trivial PATCH V2] treewide: Align function definition open/close braces

[Rafael J. Wysocki]

On Wed, Mar 21, 2018 at 11:09 PM, Joe Perches <j...@perches.com> wrote:
> Some functions definitions have either the initial open brace and/or
> the closing brace outside of column 1.
>
> Move those braces to column 1.
>
> This allows various function analyzers like gnu complexity to work
> properly for these modified functions.
>
> Signed-off-by: Joe Perches <j...@perches.com>
> Acked-by: Andy Shevchenko <andy.shevche...@gmail.com>
> Acked-by: Paul Moore <p...@paul-moore.com>
> Acked-by: Alex Deucher <alexander.deuc...@amd.com>
> Acked-by: Dave Chinner <dchin...@redhat.com>
> Reviewed-by: Darrick J. Wong <darrick.w...@oracle.com>
> Acked-by: Alexandre Belloni <alexandre.bell...@free-electrons.com>
> Acked-by: Martin K. Petersen <martin.peter...@oracle.com>
> Acked-by: Takashi Iwai <ti...@suse.de>
> Acked-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
> ---
>
> git diff -w still shows no difference.
>
> This patch was sent but December and not applied.
>
> As the trivial maintainer seems not active, it'd be nice if
> Andrew Morton picks this up.
>
> V2: Remove fs/xfs/libxfs/xfs_alloc.c as it's updated and remerge the rest
>
>  arch/x86/include/asm/atomic64_32.h   |  2 +-
>  drivers/acpi/custom_method.c |  2 +-
>  drivers/acpi/fan.c   |  2 +-

For the ACPI changes:

Acked-by: Rafael J. Wysocki <rafael.j.wyso...@intel.com>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [trivial PATCH] treewide: Align function definition open/close braces

[Rafael J. Wysocki]

(xfs_sb_version_hascrc(>m_sb)) {
> @@ -2449,8 +2449,7 @@ xfs_agf_verify(
>be32_to_cpu(agf->agf_refcount_level) > XFS_BTREE_MAXLEVELS))
>   return false;
>  
> - return true;;
> -
> + return true;
>  }
>  
>  static void
> diff --git a/fs/xfs/xfs_export.c b/fs/xfs/xfs_export.c
> index fe1bfee35898..7d5c355d78b5 100644
> --- a/fs/xfs/xfs_export.c
> +++ b/fs/xfs/xfs_export.c
> @@ -122,7 +122,7 @@ xfs_nfs_get_inode(
>   struct super_block  *sb,
>   u64 ino,
>   u32 generation)
> - {
> +{
>   xfs_mount_t *mp = XFS_M(sb);
>   xfs_inode_t *ip;
>   int error;
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 227db99b0f19..d97e8f0f73ca 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -443,15 +443,15 @@ static int audit_set_failure(u32 state)
>   * Drop any references inside the auditd connection tracking struct and free
>   * the memory.
>   */
> - static void auditd_conn_free(struct rcu_head *rcu)
> - {
> +static void auditd_conn_free(struct rcu_head *rcu)
> +{
>   struct auditd_connection *ac;
>  
>   ac = container_of(rcu, struct auditd_connection, rcu);
>   put_pid(ac->pid);
>   put_net(ac->net);
>   kfree(ac);
> - }
> +}
>  
>  /**
>   * auditd_set - Set/Reset the auditd connection state
> diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
> index ad1d6164e946..50f44b7b2b32 100644
> --- a/kernel/trace/trace_printk.c
> +++ b/kernel/trace/trace_printk.c
> @@ -196,7 +196,7 @@ struct notifier_block module_trace_bprintk_format_nb = {
>  };
>  
>  int __trace_bprintk(unsigned long ip, const char *fmt, ...)
> - {
> +{
>   int ret;
>   va_list ap;
>  
> @@ -214,7 +214,7 @@ int __trace_bprintk(unsigned long ip, const char *fmt, 
> ...)
>  EXPORT_SYMBOL_GPL(__trace_bprintk);
>  
>  int __ftrace_vbprintk(unsigned long ip, const char *fmt, va_list ap)
> - {
> +{
>   if (unlikely(!fmt))
>   return 0;
>  
> diff --git a/lib/raid6/sse2.c b/lib/raid6/sse2.c
> index 1d2276b007ee..8191e1d0d2fb 100644
> --- a/lib/raid6/sse2.c
> +++ b/lib/raid6/sse2.c
> @@ -91,7 +91,7 @@ static void raid6_sse21_gen_syndrome(int disks, size_t 
> bytes, void **ptrs)
>  
>  static void raid6_sse21_xor_syndrome(int disks, int start, int stop,
>size_t bytes, void **ptrs)
> - {
> +{
>   u8 **dptr = (u8 **)ptrs;
>   u8 *p, *q;
>   int d, z, z0;
> @@ -200,9 +200,9 @@ static void raid6_sse22_gen_syndrome(int disks, size_t 
> bytes, void **ptrs)
>   kernel_fpu_end();
>  }
>  
> - static void raid6_sse22_xor_syndrome(int disks, int start, int stop,
> +static void raid6_sse22_xor_syndrome(int disks, int start, int stop,
>size_t bytes, void **ptrs)
> - {
> +{
>   u8 **dptr = (u8 **)ptrs;
>   u8 *p, *q;
>   int d, z, z0;
> @@ -265,7 +265,7 @@ static void raid6_sse22_gen_syndrome(int disks, size_t 
> bytes, void **ptrs)
>  
>   asm volatile("sfence" : : : "memory");
>   kernel_fpu_end();
> - }
> +}
>  
>  const struct raid6_calls raid6_sse2x2 = {
>   raid6_sse22_gen_syndrome,
> @@ -366,9 +366,9 @@ static void raid6_sse24_gen_syndrome(int disks, size_t 
> bytes, void **ptrs)
>   kernel_fpu_end();
>  }
>  
> - static void raid6_sse24_xor_syndrome(int disks, int start, int stop,
> +static void raid6_sse24_xor_syndrome(int disks, int start, int stop,
>size_t bytes, void **ptrs)
> - {
> +{
>   u8 **dptr = (u8 **)ptrs;
>   u8 *p, *q;
>   int d, z, z0;
> @@ -471,7 +471,7 @@ static void raid6_sse24_gen_syndrome(int disks, size_t 
> bytes, void **ptrs)
>   }
>   asm volatile("sfence" : : : "memory");
>   kernel_fpu_end();
> - }
> +}
>  
>  
>  const struct raid6_calls raid6_sse2x4 = {
> diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c
> index 0c11f434a374..ec619f51d336 100644
> --- a/sound/soc/fsl/fsl_dma.c
> +++ b/sound/soc/fsl/fsl_dma.c
> @@ -879,7 +879,7 @@ static const struct snd_pcm_ops fsl_dma_ops = {
>  };
>  
>  static int fsl_soc_dma_probe(struct platform_device *pdev)
> - {
> +{
>   struct dma_object *dma;
>   struct device_node *np = pdev->dev.of_node;
>   struct device_node *ssi_np;
> 
> --

Acked-by: Rafael J. Wysocki <rafael.j.wyso...@intel.com>

for the ACPI part.

Thanks!

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn

[Rafael J. Wysocki]

On Fri, Feb 17, 2017 at 8:11 AM, Joe Perches  wrote:
> There are ~4300 uses of pr_warn and ~250 uses of the older
> pr_warning in the kernel source tree.
>
> Make the use of pr_warn consistent across all kernel files.
>
> This excludes all files in tools/ as there is a separate
> define pr_warning for that directory tree and pr_warn is
> not used in tools/.
>
> Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing.

Sorry about asking if that has been asked already.

Wouldn't it be slightly less intrusive to simply redefined
pr_warning() as a synonym for pr_warn()?

Thanks,
Rafael
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx