Re: [PATCH] r8152: disable rx checksum offload on Dell TB dock
> On 23 Nov 2017, at 3:58 PM, Greg KH wrote:
>
> On Thu, Nov 23, 2017 at 01:38:38AM -0500, Kai-Heng Feng wrote:
>> r8153 on Dell TB dock corrupts rx packets.
>>
>> The root cause is not found yet, but disabling rx checksumming can
>> workaround the issue. We can use this connection to decide if it's
>> a Dell TB dock:
>> Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
>>
>> BugLink: https://bugs.launchpad.net/bugs/1729674
>> Cc: Mario Limonciello
>> Signed-off-by: Kai-Heng Feng
>> ---
>> drivers/net/usb/r8152.c | 33 -
>> 1 file changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
>> index d51d9abf7986..58b80b5e7803 100644
>> --- a/drivers/net/usb/r8152.c
>> +++ b/drivers/net/usb/r8152.c
>> @@ -27,6 +27,8 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>>
>> /* Information for net-next */
>> #define NETNEXT_VERSION "09"
>> @@ -5135,6 +5137,35 @@ static u8 rtl_get_version(struct usb_interface *intf)
>> return version;
>> }
>>
>> +/* Ethernet on Dell TB 15/16 dock is connected this way:
>> + * Realtek r8153 <-> SMSC hub <-> ASMedia XHCI controller
>> + * We use this connection to make sure r8153 is on the Dell TB dock.
>> + */
>> +static bool check_dell_tb_dock(struct usb_device *udev)
>> +{
>> +struct usb_device *hub = udev->parent;
>> +struct usb_device *root_hub;
>> +struct pci_dev *controller;
>> +
>> +if (!hub)
>> +return false;
>> +
>> +if (!(le16_to_cpu(hub->descriptor.idVendor) == 0x0424 &&
>> + le16_to_cpu(hub->descriptor.idProduct) == 0x5537))
>> +return false;
>> +
>> +root_hub = hub->parent;
>> +if (!root_hub || root_hub->parent)
>> +return false;
>> +
>> +controller = to_pci_dev(bus_to_hcd(root_hub->bus)->self.controller);
>
> That's a very scary, and dangerous, cast. You can not ever be sure that
> the hub really is a "root hub" like this.
What I want to do here is to finding this connection:
Realtek r8153 <-> SMSC hub (USD ID: 0424:5537) <->
ASMedia XHCI controller (PCI ID: 1b21:1142).
Is there a safer way to do this?
>
>> +if (controller->vendor == 0x1b21 && controller->device == 0x1142)
>> +return true;
>
> Why can't you just look at the USB device itself and go off of a quirk
> in it? Something like a version or string or something else?
I have a r8153 <-> USB 3.0 dongle which work just fine. I can’t find any
information to differentiate them. Hence I want to use the connection to
identify if r8153 is on a Dell TB dock.
>
> This sounds like a USB host controller issue, not a USB device issue,
> can't we fix the "real" problem here instead of this crazy work-around?
Yes. From what I know, ASMedia is working on it, but not sure how long it
will take. In the meantime, I’d like to workaround this issue for the users.
> Odds are any device plugged into the hub should have the same issue,
> right?
Actually no.
I just plugged r8153 dongle into the same hub, surprisingly the issue
doesn’t happen in this scenario.
Kai-Heng
>
> thanks,
>
> greg k-h
Re: [PATCH v2] usb: core: Add "quirks" parameter for usbcore
> On 6 Dec 2017, at 10:10 PM, Greg KH wrote: > > On Wed, Dec 06, 2017 at 06:26:21PM +0800, Kai-Heng Feng wrote: >> Trying quirks in usbcore needs to rebuild the driver or the entire >> kernel if it's builtin. It can save a lot of time if usbcore has similar >> ability like "usbhid.quirks=" and "usb-storage.quirks=". >> >> Rename the original quirk detection function to "static" as we introduce >> this new "dynamic" function. >> >> Now users can use "usbcore.quirks=" as short term workaround before the >> next kernel release. >> >> This is inspired by usbhid and usb-storage. >> >> Signed-off-by: Kai-Heng Feng >> --- >> v2: use in-kernel tolower() function. >> >> Documentation/admin-guide/kernel-parameters.txt | 55 + >> drivers/usb/core/quirks.c | 100 >> ++-- >> include/linux/usb/quirks.h | 2 + >> 3 files changed, 151 insertions(+), 6 deletions(-) >> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt >> b/Documentation/admin-guide/kernel-parameters.txt >> index 6571fbfdb2a1..d42fb278320b 100644 >> --- a/Documentation/admin-guide/kernel-parameters.txt >> +++ b/Documentation/admin-guide/kernel-parameters.txt >> @@ -4302,6 +4302,61 @@ >> >> usbcore.nousb [USB] Disable the USB subsystem >> >> +usbcore.quirks= >> +[USB] A list of quirks entries to supplement or >> +override the built-in usb core quirk list. List >> +entries are separated by commas. Each entry has >> +the form VID:PID:Flags where VID and PID are Vendor >> +and Product ID values (4-digit hex numbers) and >> +Flags is a set of characters, each corresponding >> +to a common usb core quirk flag as follows: >> +a = USB_QUIRK_STRING_FETCH_255 (string >> +descriptors must not be fetched using >> +a 255-byte read); >> +b = USB_QUIRK_RESET_RESUME (device can't resume >> +correctly so reset it instead); >> +c = USB_QUIRK_NO_SET_INTF (device can't handle >> +Set-Interface requests); >> +d = USB_QUIRK_CONFIG_INTF_STRINGS (device can't >> +handle its Configuration or Interface >> +strings); >> +e = USB_QUIRK_RESET (device can't be reset >> +(e.g morph devices), don't use reset); >> +f = USB_QUIRK_HONOR_BNUMINTERFACES (device has >> +more interface descriptions than the >> +bNumInterfaces count, and can't handle >> +talking to these interfaces); >> +g = USB_QUIRK_DELAY_INIT (device needs a pause >> +during initialization, after we read >> +the device descriptor); >> +h = USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL (For >> +high speed and super speed interrupt >> +endpoints, the USB 2.0 and USB 3.0 spec >> +require the interval in microframes (1 >> +microframe = 125 microseconds) to be >> +calculated as interval = 2 ^ >> +(bInterval-1). >> +Devices with this quirk report their >> +bInterval as the result of this >> +calculation instead of the exponent >> +variable used in the calculation); >> +i = USB_QUIRK_DEVICE_QUALIFIER (device can't >> +handle device_qualifier descriptor >> +requests); >> +j = USB_QUIRK_IGNORE_REMOTE_WAKEUP (device >> +generates spurious wakeup, ignore >> +remote wakeu
[PATCH] Input: elan_i2c - add ELAN060C to the ACPI table
ELAN060C touchpad uses elan_i2c as its driver. It can be
found on Lenovo ideapad 320-14AST.
BugLink: https://bugs.launchpad.net/bugs/1727544
Signed-off-by: Kai-Heng Feng
---
drivers/input/mouse/elan_i2c_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/mouse/elan_i2c_core.c
b/drivers/input/mouse/elan_i2c_core.c
index 6d6b092e2da9..d6135900da64 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -1258,6 +1258,7 @@ static const struct acpi_device_id elan_acpi_id[] = {
{ "ELAN0605", 0 },
{ "ELAN0609", 0 },
{ "ELAN060B", 0 },
+ { "ELAN060C", 0 },
{ "ELAN0611", 0 },
{ "ELAN1000", 0 },
{ }
--
2.14.1
Re: [PATCH] r8152: disable rx checksum offload on Dell TB dock
> On 23 Nov 2017, at 5:24 PM, Greg KH wrote: > > On Thu, Nov 23, 2017 at 04:53:41PM +0800, Kai Heng Feng wrote: >> >> What I want to do here is to finding this connection: >> Realtek r8153 <-> SMSC hub (USD ID: 0424:5537) <-> >> ASMedia XHCI controller (PCI ID: 1b21:1142). >> >> Is there a safer way to do this? > > Nope! You can't do that at all from within a USB driver, sorry. As you > really should not care at all :) Got it :) The r8153 in Dell TB dock has version information, RTL_VER_05. We can use it to check for workaround, but many working RTL_VER_05 devices will also be affected. Do you think it’s an acceptable compromise? >> I have a r8153 <-> USB 3.0 dongle which work just fine. I can’t find any >> information to differentiate them. Hence I want to use the connection to >> identify if r8153 is on a Dell TB dock. > > Are you sure there is nothing different in the version or release number > of the device? 'lsusb -v' shows the exact same information for both > devices? Yes. I attached `lsusb -v` for r8153 on Dell TB dock, on a RJ45 <-> USB 3.0 dongle, and on a RJ45 <-> USB Type-C dongle. >> Yes. From what I know, ASMedia is working on it, but not sure how long it >> will take. In the meantime, I’d like to workaround this issue for the users. > > Again, it's a host controller bug, it should be fixed there, don't try > to paper over the real issue in different individual drivers. > > I think I've seen various patches on the linux-usb list for this > controller already, have you tried them? Yes. These patches are all in mainline Linux now. >> Actually no. >> I just plugged r8153 dongle into the same hub, surprisingly the issue >> doesn’t happen in this scenario. > > Then something seems to be wrong with the device itself, as that would > be the same exact electrical/logical path, right? I have no idea why externally plugged one doesn’t have this issue. Maybe it’s related how it’s wired inside the Dell TB dock... Kai-Heng lsusb-a Description: Binary data lsusb-c Description: Binary data lsusb-dock Description: Binary data > thanks, > > greg k-h
Re: [PATCH] r8152: disable rx checksum offload on Dell TB dock
> On 24 Nov 2017, at 4:28 PM, Greg KH wrote: > > The bcdDevice is different between the dock device and the "real" > device, why not use that? Yea, I’ll poke around and see if bcdDevice alone can be a good predicate. > Then there is still a bug. Who as ASMedia is working on this, have they > posted anything to the linux-usb mailing list about it? I think they are doing this internally. I’ll advice them to ask questions here if they encounter any problem. > Maybe. Have you tried using usbmon to see what the data streams are for > the two devices and where they have problems and diverge? Is the dock > device doing something different in response to something from the host > that the "real" device does not do? No I haven’t. Not really sure how do debug network packets over USB. I’ll do some research on the topic. Kai-Heng
Re: [PATCH] r8152: disable rx checksum offload on Dell TB dock
> Also the MAC address is different, can you just trigger off of Dell's > MAC address space instead of the address space of the dongle device? A really good idea, never thought of this. Thanks for the hint :) Still, I need to ask Dell folks to get all the answers. Kai-Heng
[PATCH v2] ALSA: hda: Balance runtime/system PM if direct-complete is disabled
After hibernation, HDA controller can't be runtime-suspended after
commit 215a22ed31a1 ("ALSA: hda: Refactor codjc PM to use
direct-complete optimization"), which enables direct-complete for HDA
codec.
The HDA codec driver didn't expect direct-complete will be disabled
after it returns a positive value from prepare() callback. However,
there are some places that PM core can disable direct-complete. For
instance, system hibernation or when codec has subordinates like LEDs.
So if the codec is prepared for direct-complete but PM core still calls
codec's suspend or freeze callback, partially revert the commit and take
the original approach, which uses pm_runtime_force_*() helpers to
ensure PM refcount are balanced. Meanwhile, still keep prepare() and
complete() callbacks to enable direct-complete and request a resume for
jack detection, respectively.
Reported-by: Kenneth R. Crudup
Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete
optimization")
Signed-off-by: Kai-Heng Feng
---
v2:
- Use pm_runtime_force_*() helpers to avoid suspend/resume ping pong.
sound/pci/hda/hda_codec.c | 24 +++-
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 687216e74526..eec1775dfffe 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -2934,7 +2934,7 @@ static void hda_call_codec_resume(struct hda_codec *codec)
snd_hdac_leave_pm(>core);
}
-static int hda_codec_suspend(struct device *dev)
+static int hda_codec_runtime_suspend(struct device *dev)
{
struct hda_codec *codec = dev_to_hda_codec(dev);
unsigned int state;
@@ -2953,7 +2953,7 @@ static int hda_codec_suspend(struct device *dev)
return 0;
}
-static int hda_codec_resume(struct device *dev)
+static int hda_codec_runtime_resume(struct device *dev)
{
struct hda_codec *codec = dev_to_hda_codec(dev);
@@ -2968,16 +2968,6 @@ static int hda_codec_resume(struct device *dev)
return 0;
}
-static int hda_codec_runtime_suspend(struct device *dev)
-{
- return hda_codec_suspend(dev);
-}
-
-static int hda_codec_runtime_resume(struct device *dev)
-{
- return hda_codec_resume(dev);
-}
-
#endif /* CONFIG_PM */
#ifdef CONFIG_PM_SLEEP
@@ -2998,31 +2988,31 @@ static void hda_codec_pm_complete(struct device *dev)
static int hda_codec_pm_suspend(struct device *dev)
{
dev->power.power_state = PMSG_SUSPEND;
- return hda_codec_suspend(dev);
+ return pm_runtime_force_suspend(dev);
}
static int hda_codec_pm_resume(struct device *dev)
{
dev->power.power_state = PMSG_RESUME;
- return hda_codec_resume(dev);
+ return pm_runtime_force_resume(dev);
}
static int hda_codec_pm_freeze(struct device *dev)
{
dev->power.power_state = PMSG_FREEZE;
- return hda_codec_suspend(dev);
+ return pm_runtime_force_suspend(dev);
}
static int hda_codec_pm_thaw(struct device *dev)
{
dev->power.power_state = PMSG_THAW;
- return hda_codec_resume(dev);
+ return pm_runtime_force_resume(dev);
}
static int hda_codec_pm_restore(struct device *dev)
{
dev->power.power_state = PMSG_RESTORE;
- return hda_codec_resume(dev);
+ return pm_runtime_force_resume(dev);
}
#endif /* CONFIG_PM_SLEEP */
--
2.29.2
Re: [PATCH] ACPI / device_sysfs: Use OF_MODALIAS for "compatible" modalias
On Tue, Jan 19, 2021 at 6:34 PM Greg Kroah-Hartman wrote: > > On Tue, Jan 19, 2021 at 11:41:59AM +0200, Andy Shevchenko wrote: > > On Tue, Jan 19, 2021 at 04:41:48PM +0800, Kai-Heng Feng wrote: > > > On Tue, Jan 19, 2021 at 4:27 PM Greg Kroah-Hartman > > > wrote: > > > > On Tue, Jan 19, 2021 at 04:15:13PM +0800, Kai-Heng Feng wrote: > > > > ... > > > > > > Who will use OF_MODALIAS and where have you documented it? > > > > > > After this lands in mainline, I'll modify the pull request for systemd > > > to add a new rule for OF_MODALIAS. > > > I'll modify the comment on the function to document the change. > > > > I'm wondering why to have two fixes in two places instead of fixing udev to > > understand multiple MODALIAS= events? > > It's not a matter of multiple events, it's a single event with a > key/value pair with duplicate keys and different values. > > What is this event with different values supposed to be doing in > userspace? Do you want multiple invocations of `modprobe` or something > else? > > Usually a "device" only has a single "signature" that modprobe uses to > look up the correct module for. Modules can support any number of > device signatures, but traditionally it is odd to think that a device > itself can be supported by multiple modules, which is what you are > saying is happening here. > > So what should userspace do with this, and why does a device need to > have multiple module alias signatures? >From the original use case [1], I think the "compatible" modalias should be enough. Andy and Mika, what do you think? Can we remove the ACPI modalias for this case? [1] https://lwn.net/Articles/612062/ Kai-Heng > > thanks, > > greg k-h
Re: Multiple MODALIAS= in uevent file confuses userspace
On Sat, Jan 9, 2021 at 12:25 AM Kai-Heng Feng
wrote:
>
> Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
> "compatible" is present") creates two modaliases for certain ACPI
> devices. However userspace (systemd-udevd in this case) assumes uevent
> file doesn't have duplicated keys, so two "MODALIAS=" breaks the
> assumption.
>
> Based on the assumption, systemd-udevd internally uses hashmap to
> store each line of uevent file, so the second modalias always replaces
> the first modalias.
>
> My attempt [1] is to add a new key, "MODALIAS1" for the second
> modalias. This brings up the question of whether each key in uevent
> file is unique. If it's no unique, this may break may userspace.
Does anyone know if there's any user of the second modalias?
If there's no user of the second one, can we change it to OF_MODALIAS
or COMPAT_MODALIAS?
Kai-Heng
>
> [1] https://github.com/systemd/systemd/pull/18163
>
> Kai-Heng
[PATCH] ACPI / device_sysfs: Prefer "compatible" modalias
Commit 8765c5ba1949 ("ACPI / scan: Rework modalias creation when
"compatible" is present") may create two "MODALIAS=" in uevent file if
conditions are met.
This breaks systemd-udevd, which assumes each "key" in uevent file is
unique. The internal implementation of systemd-udevd overwrites the
first MODALIAS with the second one, so its kmod rule doesn't load driver
for the first MODALIAS.
So if both ACPI modalias and OF modalias are present, use the latter
one to ensure there's only one MODALIAS.
Reference: https://github.com/systemd/systemd/pull/18163
Cc: AceLan Kao
Cc: "Rafael J. Wysocki"
Cc: Greg Kroah-Hartman
Cc: Andy Shevchenko
Suggested-by: Mika Westerberg
Fixes: 8765c5ba1949 ("ACPI / scan: Rework modalias creation when "compatible"
is present")
Signed-off-by: Kai-Heng Feng
---
drivers/acpi/device_sysfs.c | 20 ++--
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 96869f1538b9..bfca116482b8 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -251,20 +251,12 @@ int __acpi_device_uevent_modalias(struct acpi_device
*adev,
if (add_uevent_var(env, "MODALIAS="))
return -ENOMEM;
- len = create_pnp_modalias(adev, >buf[env->buflen - 1],
- sizeof(env->buf) - env->buflen);
- if (len < 0)
- return len;
-
- env->buflen += len;
- if (!adev->data.of_compatible)
- return 0;
-
- if (len > 0 && add_uevent_var(env, "MODALIAS="))
- return -ENOMEM;
-
- len = create_of_modalias(adev, >buf[env->buflen - 1],
-sizeof(env->buf) - env->buflen);
+ if (adev->data.of_compatible)
+ len = create_of_modalias(adev, >buf[env->buflen - 1],
+sizeof(env->buf) - env->buflen);
+ else
+ len = create_pnp_modalias(adev, >buf[env->buflen - 1],
+ sizeof(env->buf) - env->buflen);
if (len < 0)
return len;
--
2.29.2
Re: [PATCH] ALSA: hda: Balance runtime/system PM if direct-complete is disabled
On Mon, Jan 18, 2021 at 9:21 PM Takashi Iwai wrote:
>
> On Mon, 18 Jan 2021 14:09:36 +0100,
> Kai-Heng Feng wrote:
> >
> > HDA controller can't be runtime-suspended after commit 215a22ed31a1
> > ("ALSA: hda: Refactor codjc PM to use direct-complete optimization"),
> > which enables direct-complete for HDA codec.
> >
> > The HDA codec driver didn't expect direct-complete will be disabled
> > after it returns a positive value from prepare() callback. However,
> > there are some places that PM core can disable direct-complete. For
> > instance, system hibernation or when codec has subordinates like LEDs.
>
> Hmm. This sounds rather like the approach using the direct-complete
> isn't well suited for the purpose? The increasing number of
> regression reports worries me.
Direct-complete works fine on HDA controller but so far not so on HDA codec.
I think the main reason is that the codec doesn't have the middle
layer to handle the detail, while HDA controller has PCI bus to deal
with them.
>
> > So if a device is prepared for direct-complete but PM core still calls
> > codec's suspend or freeze callback, resume the device to keep PM
> > operations balanced.
>
> I find the ping-pong of the resume/suspend there a bit odd. It's no
> refcount management but it invokes the real resume there, which is
> involved with lots of operations.
Yes. I'll find a better approach to address this.
>
> Can we rather skip the hda_codec_suspend() call instead (while
> changing dev->power.power_state)?
Maybe we can revert the most of the commit, and just leave
hda_codec_pm_complete(), which is the most relevant part of the patch.
Let me test a bit and send a new patch. Let me know if you don't like
this approach.
A question a bit unrelated to the discussion - how does
snd_hdac_power_up_pm() work for concurrent resume?
It can work for recursive call, but what if there are concurrent
resume request, but pm_runtime_get_sync() is still running?
The second call may access the codec which hasn't completed resume.
Kai-Heng
>
>
> thanks,
>
> Takashi
>
> > Reported-by: Kenneth R. Crudup
> > Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete
> > optimization")
> > Signed-off-by: Kai-Heng Feng
> > ---
> > sound/pci/hda/hda_codec.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> > index 687216e74526..0afbced979df 100644
> > --- a/sound/pci/hda/hda_codec.c
> > +++ b/sound/pci/hda/hda_codec.c
> > @@ -2997,6 +2997,9 @@ static void hda_codec_pm_complete(struct device *dev)
> >
> > static int hda_codec_pm_suspend(struct device *dev)
> > {
> > + if (pm_runtime_status_suspended(dev))
> > + pm_runtime_resume(dev);
> > +
> > dev->power.power_state = PMSG_SUSPEND;
> > return hda_codec_suspend(dev);
> > }
> > @@ -3009,6 +3012,9 @@ static int hda_codec_pm_resume(struct device *dev)
> >
> > static int hda_codec_pm_freeze(struct device *dev)
> > {
> > + if (pm_runtime_status_suspended(dev))
> > + pm_runtime_resume(dev);
> > +
> > dev->power.power_state = PMSG_FREEZE;
> > return hda_codec_suspend(dev);
> > }
> > --
> > 2.29.2
> >
Re: [PATCH] HID: multitouch: Apply MT_QUIRK_CONFIDENCE quirk for multi-input devices
Hi,
On Mon, Jan 18, 2021 at 10:41 PM Benjamin Tissoires
wrote:
>
> Hi,
>
> On Mon, Jan 18, 2021 at 2:45 PM Kai-Heng Feng
> wrote:
> >
> > Palm ejection stops working on some Elan and Synaptics touchpad after
> > commit 40d5bb87377a ("HID: multitouch: enable multi-input as a quirk for
> > some devices").
> >
> > The commit changes the mt_class from MT_CLS_WIN_8 to
> > MT_CLS_WIN_8_FORCE_MULTI_INPUT, so MT_QUIRK_CONFIDENCE isn't applied
> > anymore.
> >
> > So also apply the quirk since MT_CLS_WIN_8_FORCE_MULTI_INPUT is
> > essentially MT_CLS_WIN_8.
> >
> > Fixes: 40d5bb87377a ("HID: multitouch: enable multi-input as a quirk for
> > some devices")
> > Signed-off-by: Kai-Heng Feng
>
> Thanks for the patch.
>
> IIt seems I was too lazy to write a regression test for it, and this
> strikes back.
> Can you also work on a regression test for this at
> https://gitlab.freedesktop.org/libevdev/hid-tools ?
Of course. I'll do it later this week. Currently I have both affected
Elan and Synaptics touchpad in hand.
Will this be a blocker of getting the patch merged?
Kai-Heng
>
> Cheers,
> Benjamin
>
>
>
>
> > ---
> > drivers/hid/hid-multitouch.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> > index 0743ef51d3b2..8429ebe7097e 100644
> > --- a/drivers/hid/hid-multitouch.c
> > +++ b/drivers/hid/hid-multitouch.c
> > @@ -758,7 +758,8 @@ static int mt_touch_input_mapping(struct hid_device
> > *hdev, struct hid_input *hi,
> > MT_STORE_FIELD(inrange_state);
> > return 1;
> > case HID_DG_CONFIDENCE:
> > - if (cls->name == MT_CLS_WIN_8 &&
> > + if ((cls->name == MT_CLS_WIN_8 ||
> > +cls->name == MT_CLS_WIN_8_FORCE_MULTI_INPUT) &&
> > (field->application == HID_DG_TOUCHPAD ||
> > field->application == HID_DG_TOUCHSCREEN))
> > app->quirks |= MT_QUIRK_CONFIDENCE;
> > --
> > 2.29.2
> >
>
[PATCH] ALSA: hda: Balance runtime/system PM if direct-complete is disabled
HDA controller can't be runtime-suspended after commit 215a22ed31a1
("ALSA: hda: Refactor codjc PM to use direct-complete optimization"),
which enables direct-complete for HDA codec.
The HDA codec driver didn't expect direct-complete will be disabled
after it returns a positive value from prepare() callback. However,
there are some places that PM core can disable direct-complete. For
instance, system hibernation or when codec has subordinates like LEDs.
So if a device is prepared for direct-complete but PM core still calls
codec's suspend or freeze callback, resume the device to keep PM
operations balanced.
Reported-by: Kenneth R. Crudup
Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete
optimization")
Signed-off-by: Kai-Heng Feng
---
sound/pci/hda/hda_codec.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 687216e74526..0afbced979df 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -2997,6 +2997,9 @@ static void hda_codec_pm_complete(struct device *dev)
static int hda_codec_pm_suspend(struct device *dev)
{
+ if (pm_runtime_status_suspended(dev))
+ pm_runtime_resume(dev);
+
dev->power.power_state = PMSG_SUSPEND;
return hda_codec_suspend(dev);
}
@@ -3009,6 +3012,9 @@ static int hda_codec_pm_resume(struct device *dev)
static int hda_codec_pm_freeze(struct device *dev)
{
+ if (pm_runtime_status_suspended(dev))
+ pm_runtime_resume(dev);
+
dev->power.power_state = PMSG_FREEZE;
return hda_codec_suspend(dev);
}
--
2.29.2
[PATCH] HID: multitouch: Apply MT_QUIRK_CONFIDENCE quirk for multi-input devices
Palm ejection stops working on some Elan and Synaptics touchpad after
commit 40d5bb87377a ("HID: multitouch: enable multi-input as a quirk for
some devices").
The commit changes the mt_class from MT_CLS_WIN_8 to
MT_CLS_WIN_8_FORCE_MULTI_INPUT, so MT_QUIRK_CONFIDENCE isn't applied
anymore.
So also apply the quirk since MT_CLS_WIN_8_FORCE_MULTI_INPUT is
essentially MT_CLS_WIN_8.
Fixes: 40d5bb87377a ("HID: multitouch: enable multi-input as a quirk for some
devices")
Signed-off-by: Kai-Heng Feng
---
drivers/hid/hid-multitouch.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 0743ef51d3b2..8429ebe7097e 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -758,7 +758,8 @@ static int mt_touch_input_mapping(struct hid_device *hdev,
struct hid_input *hi,
MT_STORE_FIELD(inrange_state);
return 1;
case HID_DG_CONFIDENCE:
- if (cls->name == MT_CLS_WIN_8 &&
+ if ((cls->name == MT_CLS_WIN_8 ||
+cls->name == MT_CLS_WIN_8_FORCE_MULTI_INPUT) &&
(field->application == HID_DG_TOUCHPAD ||
field->application == HID_DG_TOUCHSCREEN))
app->quirks |= MT_QUIRK_CONFIDENCE;
--
2.29.2
Re: [PATCH] HID: multitouch: Apply MT_QUIRK_CONFIDENCE quirk for multi-input devices
On Tue, Jan 19, 2021 at 1:45 AM Kai-Heng Feng
wrote:
>
> Hi,
>
> On Mon, Jan 18, 2021 at 10:41 PM Benjamin Tissoires
> wrote:
> >
> > Hi,
> >
> > On Mon, Jan 18, 2021 at 2:45 PM Kai-Heng Feng
> > wrote:
> > >
> > > Palm ejection stops working on some Elan and Synaptics touchpad after
> > > commit 40d5bb87377a ("HID: multitouch: enable multi-input as a quirk for
> > > some devices").
> > >
> > > The commit changes the mt_class from MT_CLS_WIN_8 to
> > > MT_CLS_WIN_8_FORCE_MULTI_INPUT, so MT_QUIRK_CONFIDENCE isn't applied
> > > anymore.
> > >
> > > So also apply the quirk since MT_CLS_WIN_8_FORCE_MULTI_INPUT is
> > > essentially MT_CLS_WIN_8.
> > >
> > > Fixes: 40d5bb87377a ("HID: multitouch: enable multi-input as a quirk for
> > > some devices")
> > > Signed-off-by: Kai-Heng Feng
> >
> > Thanks for the patch.
> >
> > IIt seems I was too lazy to write a regression test for it, and this
> > strikes back.
> > Can you also work on a regression test for this at
> > https://gitlab.freedesktop.org/libevdev/hid-tools ?
>
> Of course. I'll do it later this week. Currently I have both affected
> Elan and Synaptics touchpad in hand.
> Will this be a blocker of getting the patch merged?
I made a pull request:
https://gitlab.freedesktop.org/libevdev/hid-tools/-/merge_requests/105
The tests don't pass for both Elan and Synaptics device, but let's fix it there.
Kai-Heng
>
> Kai-Heng
>
> >
> > Cheers,
> > Benjamin
> >
> >
> >
> >
> > > ---
> > > drivers/hid/hid-multitouch.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> > > index 0743ef51d3b2..8429ebe7097e 100644
> > > --- a/drivers/hid/hid-multitouch.c
> > > +++ b/drivers/hid/hid-multitouch.c
> > > @@ -758,7 +758,8 @@ static int mt_touch_input_mapping(struct hid_device
> > > *hdev, struct hid_input *hi,
> > > MT_STORE_FIELD(inrange_state);
> > > return 1;
> > > case HID_DG_CONFIDENCE:
> > > - if (cls->name == MT_CLS_WIN_8 &&
> > > + if ((cls->name == MT_CLS_WIN_8 ||
> > > +cls->name == MT_CLS_WIN_8_FORCE_MULTI_INPUT)
> > > &&
> > > (field->application == HID_DG_TOUCHPAD ||
> > > field->application ==
> > > HID_DG_TOUCHSCREEN))
> > > app->quirks |= MT_QUIRK_CONFIDENCE;
> > > --
> > > 2.29.2
> > >
> >
Re: [PATCH v2 1/3] ASoC: SOF: Intel: hda: Resume codec to do jack detection
On Tue, Jan 5, 2021 at 9:00 PM Kai Vehmanen
wrote:
>
> Hi,
>
> On Mon, 4 Jan 2021, Kai-Heng Feng wrote:
>
> > Instead of queueing jackpoll_work, runtime resume the codec to let it
> > use different jack detection methods based on jackpoll_interval.
>
> hmm, but jackpoll_work() does the same thing, right? So end result should
> be the same.
It depends on the jackpoll_interval value. But yes the end result
should be the same.
>
> > This matches SOF driver's behavior with commit a6e7d0a4bdb0 ("ALSA: hda:
> > fix jack detection with Realtek codecs when in D3"). Since SOF only uses
> > Realtek codec, we don't need any additional check for the resume.
>
> This is not quite correct. First, SOF does support any HDA codec, not just
> Realteks (see e.g. https://github.com/thesofproject/linux/issues/1807),
> and second, this doesn't really match the hda_intel.c patch you mention.
> SOF implements a more conservative approach where we basicly assume
> codec->forced_resume=1 to always apply, and do not implement support for
> codec->relaxed_resume. So the above patch doesn't fully apply to SOF as
> the design is not same.
OK, I assumed SOF always use Realtek codec, so codec->forced_resume=1
is always applied like the other patch.
I'll remove this section.
>
> > diff --git a/sound/soc/sof/intel/hda-codec.c
> > b/sound/soc/sof/intel/hda-codec.c
> > index 6875fa570c2c..df59c79cfdfc 100644
> > --- a/sound/soc/sof/intel/hda-codec.c
> > +++ b/sound/soc/sof/intel/hda-codec.c
> > @@ -93,8 +93,7 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev)
> >* has been recorded in STATESTS
> >*/
> > if (codec->jacktbl.used)
> > - schedule_delayed_work(>jackpoll_work,
> > - codec->jackpoll_interval);
> > + pm_request_resume(>core.dev);
>
> I think this change is still good. Just drop the but about Realtek
> codecs from commit message and maybe s/matches/aligns/.
OK, will do.
Kai-Heng
>
> Br, Kai
Re: [PATCH] rtw88: 8821c: Add RFE 2 support
On Wed, Aug 5, 2020 at 7:24 PM Kai-Heng Feng wrote: > > Hi Tony, > > > On Aug 5, 2020, at 19:18, Tony Chuang wrote: > > > >> 8821CE with RFE 2 isn't supported: > >> [ 12.404834] rtw_8821ce :02:00.0: rfe 2 isn't supported > >> [ 12.404937] rtw_8821ce :02:00.0: failed to setup chip efuse info > >> [ 12.404939] rtw_8821ce :02:00.0: failed to setup chip information > >> > > > > NACK > > > > The RFE type 2 should be working with some additional fixes. > > Did you tested connecting to AP with BT paired? > > No, I only tested WiFi. > > > The antenna configuration is different with RFE type 0. > > I will ask someone else to fix them. > > Then the RFE type 2 modules can be supported. > > Good to know that, I'll be patient and wait for a real fix. It's been quite some time, is support for RFE type 2 ready now? Kai-Heng > > Kai-Heng > > > > > Yen-Hsuan >
Re: [PATCH v2 3/3] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
On Tue, Jan 5, 2021 at 8:28 PM Kai Vehmanen
wrote:
>
> Hey,
>
> On Mon, 4 Jan 2021, Kai-Heng Feng wrote:
>
> > System takes a very long time to suspend after commit 215a22ed31a1
> > ("ALSA: hda: Refactor codec PM to use direct-complete optimization"):
> > [ 90.065964] PM: suspend entry (s2idle)
>
> the patch itself looks good, but can you explain a bit more in what
> conditions you hit the delay?
If both controller and codec are suspended, I can 100% reproduce the issue.
>
> I tried to reproduce the delay on multiple systems (with tip of
> tiwai/master), but with no luck. I can see hda_jackpoll_work() called, but
> at this point runtime pm has been disabled already (via
> __device_suspend()) and snd_hdac_is_power_on() will return true even when
> pm_runtime_suspended() is true as well (which is expected as runtime-pm is
> disabled at this point for system suspend). End result is codec is not
> powered up in hda_jackpoll_work() and suspend is not delayed.
On my system snd_hdac_is_power_on() calls hda_set_power_state() which
takes long time to write to (suspended) codec.
I am not sure why it doesn't power up codec on your system.
>
> The patch still seems correct. You would hit the problem you describe if
> jackpoll_interval was set to a non-zero value (not the case on most
> systems supported by SOF, but still a possibility). I'm still curious how
> you hit the problem. At minimum, we are missing a scenario in our testing.
The issue happens with zero jackpoll_interval.
Kai-Heng
>
> Br, Kai
[PATCH v2] ACPI / device_sysfs: Use OF_MODALIAS for "compatible" modalias
Commit 8765c5ba1949 ("ACPI / scan: Rework modalias creation when
"compatible" is present") may create two "MODALIAS=" in uevent file if
conditions are met.
This breaks systemd-udevd, which assumes each "key" in uevent file is
unique. The internal implementation of systemd-udevd overwrites the
first MODALIAS with the second one, so its kmod rule doesn't load driver
for the first MODALIAS.
Right now it doesn't seem to have any user relies on the second
MODALIAS, so change it to OF_MODALIAS to workaround the issue.
Reference: https://github.com/systemd/systemd/pull/18163
Fixes: 8765c5ba1949 ("ACPI / scan: Rework modalias creation when "compatible"
is present")
Signed-off-by: Kai-Heng Feng
---
v2:
Add a comment to document why it's changed.
drivers/acpi/device_sysfs.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 96869f1538b9..17483c40deeb 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -260,7 +260,11 @@ int __acpi_device_uevent_modalias(struct acpi_device *adev,
if (!adev->data.of_compatible)
return 0;
- if (len > 0 && add_uevent_var(env, "MODALIAS="))
+ /* Two "MODALIAS=" breaks how systemd-udevd handles uevent file.
+* As userspace may not be able to handle duplicated keys, add prefix
+* "OF_" to avoid the key collision.
+*/
+ if (len > 0 && add_uevent_var(env, "OF_MODALIAS="))
return -ENOMEM;
len = create_of_modalias(adev, >buf[env->buflen - 1],
--
2.29.2
[PATCH] ACPI / device_sysfs: Use OF_MODALIAS for "compatible" modalias
Commit 8765c5ba1949 ("ACPI / scan: Rework modalias creation when
"compatible" is present") may create two "MODALIAS=" in uevent file if
conditions are met.
This breaks systemd-udevd, which assumes each "key" in uevent file is
unique. The internal implementation of systemd-udevd overwrites the
first MODALIAS with the second one, so its kmod rule doesn't load driver
for the first MODALIAS.
Right now it doesn't seem to have any user relies on the second
MODALIAS, so change it to OF_MODALIAS to workaround the issue.
Reference: https://github.com/systemd/systemd/pull/18163
Fixes: 8765c5ba1949 ("ACPI / scan: Rework modalias creation when "compatible"
is present")
Cc: AceLan Kao
Cc: "Rafael J. Wysocki"
Cc: Greg Kroah-Hartman ,
Cc: Mika Westerberg ,
Cc: Andy Shevchenko
Signed-off-by: Kai-Heng Feng
---
drivers/acpi/device_sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 96869f1538b9..c92b671cb816 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -260,7 +260,7 @@ int __acpi_device_uevent_modalias(struct acpi_device *adev,
if (!adev->data.of_compatible)
return 0;
- if (len > 0 && add_uevent_var(env, "MODALIAS="))
+ if (len > 0 && add_uevent_var(env, "OF_MODALIAS="))
return -ENOMEM;
len = create_of_modalias(adev, >buf[env->buflen - 1],
--
2.29.2
Re: [PATCH] ACPI / device_sysfs: Use OF_MODALIAS for "compatible" modalias
On Tue, Jan 19, 2021 at 4:27 PM Greg Kroah-Hartman
wrote:
>
> On Tue, Jan 19, 2021 at 04:15:13PM +0800, Kai-Heng Feng wrote:
> > Commit 8765c5ba1949 ("ACPI / scan: Rework modalias creation when
> > "compatible" is present") may create two "MODALIAS=" in uevent file if
> > conditions are met.
> >
> > This breaks systemd-udevd, which assumes each "key" in uevent file is
> > unique. The internal implementation of systemd-udevd overwrites the
> > first MODALIAS with the second one, so its kmod rule doesn't load driver
> > for the first MODALIAS.
> >
> > Right now it doesn't seem to have any user relies on the second
> > MODALIAS, so change it to OF_MODALIAS to workaround the issue.
> >
> > Reference: https://github.com/systemd/systemd/pull/18163
> > Fixes: 8765c5ba1949 ("ACPI / scan: Rework modalias creation when
> > "compatible" is present")
> > Cc: AceLan Kao
> > Cc: "Rafael J. Wysocki"
> > Cc: Greg Kroah-Hartman ,
> > Cc: Mika Westerberg ,
> > Cc: Andy Shevchenko
> > Signed-off-by: Kai-Heng Feng
> > ---
> > drivers/acpi/device_sysfs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> > index 96869f1538b9..c92b671cb816 100644
> > --- a/drivers/acpi/device_sysfs.c
> > +++ b/drivers/acpi/device_sysfs.c
> > @@ -260,7 +260,7 @@ int __acpi_device_uevent_modalias(struct acpi_device
> > *adev,
> > if (!adev->data.of_compatible)
> > return 0;
> >
> > - if (len > 0 && add_uevent_var(env, "MODALIAS="))
> > + if (len > 0 && add_uevent_var(env, "OF_MODALIAS="))
>
> Who will use OF_MODALIAS and where have you documented it?
After this lands in mainline, I'll modify the pull request for systemd
to add a new rule for OF_MODALIAS.
I'll modify the comment on the function to document the change.
Kai-Heng
>
> thanks,
>
> greg k-h
Re: [PATCH 2/2] PCI: vmd: Enable ASPM for mobile platforms
Hi Bjorn,
> On Oct 3, 2020, at 06:18, Bjorn Helgaas wrote:
>
> On Wed, Sep 30, 2020 at 04:24:54PM +0800, Kai-Heng Feng wrote:
>> BIOS may not be able to program ASPM for links behind VMD, prevent Intel
>> SoC from entering deeper power saving state.
>
> It's not a question of BIOS not being *able* to configure ASPM. I
> think BIOS could do it, at least in principle, if it had a driver for
> VMD. Actually, it probably *does* include some sort of VMD code
> because it sounds like BIOS can assign some Root Ports to appear
> either as regular Root Ports or behind the VMD.
>
> Since this issue is directly related to the unusual VMD topology, I
> think it would be worth a quick recap here. Maybe something like:
>
> VMD is a Root Complex Integrated Endpoint that acts as a host bridge
> to a secondary PCIe domain. BIOS can reassign one or more Root
> Ports to appear within a VMD domain instead of the primary domain.
>
> However, BIOS may not enable ASPM for the hierarchies behind a VMD,
> ...
>
> (This is based on the commit log from 185a383ada2e ("x86/PCI: Add
> driver for Intel Volume Management Device (VMD)")).
Ok, will just copy the portion as-is if there's patch v2 :)
>
> But we still have the problem that CONFIG_PCIEASPM_DEFAULT=y means
> "use the BIOS defaults", and this patch would make it so we use the
> BIOS defaults *except* for things behind VMD.
>
> - Why should VMD be a special case?
Because BIOS doesn't handle ASPM for it so it's up to software to do the job.
In the meantime we want other devices still use the BIOS defaults to not
introduce any regression.
>
> - How would we document such a special case?
I wonder whether other devices that add PCIe domain have the same behavior?
Maybe it's not a special case at all...
I understand the end goal is to keep consistency for the entire ASPM logic.
However I can't think of any possible solution right now.
>
> - If we built with CONFIG_PCIEASPM_POWERSAVE=y, would that solve the
>SoC power state problem?
Yes.
>
> - What issues would CONFIG_PCIEASPM_POWERSAVE=y introduce?
This will break many systems, at least for the 1st Gen Ryzen desktops and
laptops.
All PCIe ASPM are not enabled by BIOS, and those systems immediately freeze
once ASPM is enabled.
Kai-Heng
>
> Link to previous discussion for the archives:
> https://lore.kernel.org/r/49a36179-d336-4a5e-8b7a-a632833ae...@canonical.com
>
>> So enable ASPM for links behind VMD to increase battery life.
>>
>> Signed-off-by: Kai-Heng Feng
>> ---
>> drivers/pci/controller/vmd.c | 22 +-
>> 1 file changed, 21 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
>> index f69ef8c89f72..058fdef9c566 100644
>> --- a/drivers/pci/controller/vmd.c
>> +++ b/drivers/pci/controller/vmd.c
>> @@ -417,6 +417,22 @@ static int vmd_find_free_domain(void)
>> return domain + 1;
>> }
>>
>> +static const struct pci_device_id vmd_mobile_bridge_tbl[] = {
>> +{ PCI_VDEVICE(INTEL, 0x9a09) },
>> +{ PCI_VDEVICE(INTEL, 0xa0b0) },
>> +{ PCI_VDEVICE(INTEL, 0xa0bc) },
>> +{ }
>> +};
>> +
>> +static int vmd_enable_aspm(struct device *dev, void *data)
>> +{
>> +struct pci_dev *pdev = to_pci_dev(dev);
>> +
>> +pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL);
>> +
>> +return 0;
>> +}
>> +
>> static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
>> {
>> struct pci_sysdata *sd = >sysdata;
>> @@ -603,8 +619,12 @@ static int vmd_enable_domain(struct vmd_dev *vmd,
>> unsigned long features)
>> * and will fail pcie_bus_configure_settings() early. It can instead be
>> * run on each of the real root ports.
>> */
>> -list_for_each_entry(child, >bus->children, node)
>> +list_for_each_entry(child, >bus->children, node) {
>> +if (pci_match_id(vmd_mobile_bridge_tbl, child->self))
>> +device_for_each_child(>self->dev, NULL,
>> vmd_enable_aspm);
>
> Wouldn't something like this be sufficient?
>
> list_for_each_entry(dev, >devices, bus_list)
>vmd_enable_aspm(dev);
>
>> pcie_bus_configure_settings(child);
>> +}
>>
>> pci_bus_add_devices(vmd->bus);
>>
>> --
>> 2.17.1
Re: [PATCH 2/2] PCI: vmd: Enable ASPM for mobile platforms
> On Oct 6, 2020, at 03:19, Bjorn Helgaas wrote:
>
> [+cc Ian, who's also working on an ASPM issue]
>
> On Tue, Oct 06, 2020 at 02:40:32AM +0800, Kai-Heng Feng wrote:
>>> On Oct 3, 2020, at 06:18, Bjorn Helgaas wrote:
>>> On Wed, Sep 30, 2020 at 04:24:54PM +0800, Kai-Heng Feng wrote:
>>>> BIOS may not be able to program ASPM for links behind VMD, prevent Intel
>>>> SoC from entering deeper power saving state.
>>>
>>> It's not a question of BIOS not being *able* to configure ASPM. I
>>> think BIOS could do it, at least in principle, if it had a driver for
>>> VMD. Actually, it probably *does* include some sort of VMD code
>>> because it sounds like BIOS can assign some Root Ports to appear
>>> either as regular Root Ports or behind the VMD.
>>>
>>> Since this issue is directly related to the unusual VMD topology, I
>>> think it would be worth a quick recap here. Maybe something like:
>>>
>>> VMD is a Root Complex Integrated Endpoint that acts as a host bridge
>>> to a secondary PCIe domain. BIOS can reassign one or more Root
>>> Ports to appear within a VMD domain instead of the primary domain.
>>>
>>> However, BIOS may not enable ASPM for the hierarchies behind a VMD,
>>> ...
>>>
>>> (This is based on the commit log from 185a383ada2e ("x86/PCI: Add
>>> driver for Intel Volume Management Device (VMD)")).
>>
>> Ok, will just copy the portion as-is if there's patch v2 :)
>>
>>> But we still have the problem that CONFIG_PCIEASPM_DEFAULT=y means
>>> "use the BIOS defaults", and this patch would make it so we use the
>>> BIOS defaults *except* for things behind VMD.
>>>
>>> - Why should VMD be a special case?
>>
>> Because BIOS doesn't handle ASPM for it so it's up to software to do
>> the job. In the meantime we want other devices still use the BIOS
>> defaults to not introduce any regression.
>>
>>> - How would we document such a special case?
>>
>> I wonder whether other devices that add PCIe domain have the same
>> behavior? Maybe it's not a special case at all...
>
> What other devices are these?
Controllers which add PCIe domain.
>
>> I understand the end goal is to keep consistency for the entire ASPM
>> logic. However I can't think of any possible solution right now.
>>
>>> - If we built with CONFIG_PCIEASPM_POWERSAVE=y, would that solve the
>>> SoC power state problem?
>>
>> Yes.
>>
>>> - What issues would CONFIG_PCIEASPM_POWERSAVE=y introduce?
>>
>> This will break many systems, at least for the 1st Gen Ryzen
>> desktops and laptops.
>>
>> All PCIe ASPM are not enabled by BIOS, and those systems immediately
>> freeze once ASPM is enabled.
>
> That indicates a defect in the Linux ASPM code. We should fix that.
> It should be safe to use CONFIG_PCIEASPM_POWERSAVE=y on every system.
On those systems ASPM are also not enabled on Windows. So I think ASPM are
disabled for a reason.
>
> Are there bug reports for these? The info we would need to start with
> includes "lspci -vv" and dmesg log (with CONFIG_PCIEASPM_DEFAULT=y).
> If a console log with CONFIG_PCIEASPM_POWERSAVE=y is available, that
> might be interesting, too. We'll likely need to add some
> instrumentation and do some experimentation, but in principle, this
> should be fixable.
Doing this is asking users to use hardware settings that ODM/OEM never tested,
and I think the risk is really high.
Kai-Heng
>
> Bjorn
[PATCH 1/2] drm/i915/dpcd_bl: Skip testing control capability with force DPCD quirk
HP DreamColor panel needs to be controlled via AUX interface. However,
it has both DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP and
DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP set, so it fails to pass
intel_dp_aux_display_control_capable() test.
Skip the test if the panel has force DPCD quirk.
Signed-off-by: Kai-Heng Feng
---
drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index acbd7eb66cbe..acf2e1c65290 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -347,9 +347,13 @@ int intel_dp_aux_init_backlight_funcs(struct
intel_connector *intel_connector)
struct intel_panel *panel = _connector->panel;
struct intel_dp *intel_dp = enc_to_intel_dp(intel_connector->encoder);
struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+ bool force_dpcd;
+
+ force_dpcd = drm_dp_has_quirk(_dp->desc, intel_dp->edid_quirks,
+ DP_QUIRK_FORCE_DPCD_BACKLIGHT);
if (i915->params.enable_dpcd_backlight == 0 ||
- !intel_dp_aux_display_control_capable(intel_connector))
+ (!force_dpcd &&
!intel_dp_aux_display_control_capable(intel_connector)))
return -ENODEV;
/*
@@ -358,9 +362,7 @@ int intel_dp_aux_init_backlight_funcs(struct
intel_connector *intel_connector)
*/
if (i915->vbt.backlight.type !=
INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE &&
- i915->params.enable_dpcd_backlight != 1 &&
- !drm_dp_has_quirk(_dp->desc, intel_dp->edid_quirks,
- DP_QUIRK_FORCE_DPCD_BACKLIGHT)) {
+ i915->params.enable_dpcd_backlight != 1 && !force_dpcd) {
drm_info(>drm,
"Panel advertises DPCD backlight support, but "
"VBT disagrees. If your backlight controls "
--
2.17.1
[PATCH 2/2] drm/dp: HP DreamColor panel brigntness fix
HP DreamColor panel, which is used by new HP ZBook Studio, needs to use
DPCD to control brightness.
Signed-off-by: Kai-Heng Feng
---
drivers/gpu/drm/drm_dp_helper.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 092c8c985911..b3d0ea1b082b 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1321,6 +1321,7 @@ static const struct edid_quirk edid_quirk_list[] = {
*/
{ MFG(0x06, 0xaf), PROD_ID(0x9b, 0x32),
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
{ MFG(0x06, 0xaf), PROD_ID(0xeb, 0x41),
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
+ { MFG(0x30, 0xe4), PROD_ID(0x61, 0x06),
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
{ MFG(0x4d, 0x10), PROD_ID(0xc7, 0x14),
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
{ MFG(0x4d, 0x10), PROD_ID(0xe6, 0x14),
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
{ MFG(0x4c, 0x83), PROD_ID(0x47, 0x41),
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
--
2.17.1
Re: [PATCH] drm/i915: Force DPCD backlight mode for HP Spectre x360 Convertible 13t-aw100
> On Apr 8, 2020, at 15:22, Jani Nikula wrote:
>
> On Tue, 07 Apr 2020, Kai-Heng Feng wrote:
>>> On Mar 27, 2020, at 19:03, Kai-Heng Feng
>>> wrote:
>>>
>>> Hi,
>>>
>>>> On Mar 23, 2020, at 13:35, Kai-Heng Feng
>>>> wrote:
>>>>
>>>> There's another OLED panel needs to use DPCD aux interface to control
>>>> backlight.
>>>>
>>>> BugLink: https://bugs.launchpad.net/bugs/1860303
>>>> Signed-off-by: Kai-Heng Feng
>>>
>>> Would it be possible to review this?
>>> I'd like to send a similar quirk for a new panel, and I want to avoid
>>> causing any merge conflict.
>>
>> Another gentle ping...
>
> Can't really review, but if you say that's needed...
>
> Acked-by: Jani Nikula
David,
Can you please merge this patch? Thanks.
Kai-Heng
>
>>
>>>
>>> Kai-Heng
>>>
>>>> ---
>>>> drivers/gpu/drm/drm_dp_helper.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_dp_helper.c
>>>> b/drivers/gpu/drm/drm_dp_helper.c
>>>> index 8ba4531e808d..a0d4314663de 100644
>>>> --- a/drivers/gpu/drm/drm_dp_helper.c
>>>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>>>> @@ -1301,6 +1301,8 @@ static const struct edid_quirk edid_quirk_list[] = {
>>>> * only supports DPCD backlight controls
>>>> */
>>>>{ MFG(0x4c, 0x83), PROD_ID(0x41, 0x41),
>>>> BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
>>>> + /* HP Spectre x360 Convertible 13t-aw100 */
>>>> + { MFG(0x4c, 0x83), PROD_ID(0x42, 0x41),
>>>> BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
>>>>/*
>>>> * Some Dell CML 2020 systems have panels support both AUX and PWM
>>>> * backlight control, and some only support AUX backlight control. All
>>>> --
>>>> 2.17.1
>>>>
>>>
>>
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] drm/i915: Force DPCD backlight mode for HP CML 2020 system
> On Apr 8, 2020, at 15:23, Jani Nikula wrote:
>
> On Tue, 07 Apr 2020, Kai-Heng Feng wrote:
>> There's another Samsung OLED panel needs to use DPCD aux interface to
>> control backlight.
>
> Acked-by: Jani Nikula
David,
Can you please merge this patch? Thanks.
Kai-Heng
>
>>
>> Signed-off-by: Kai-Heng Feng
>> ---
>> drivers/gpu/drm/drm_dp_helper.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c
>> b/drivers/gpu/drm/drm_dp_helper.c
>> index c6fbe6e6bc9d..d0cfee3b7a65 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -1299,6 +1299,8 @@ static const struct edid_quirk edid_quirk_list[] = {
>> * only supports DPCD backlight controls
>> */
>> { MFG(0x4c, 0x83), PROD_ID(0x41, 0x41),
>> BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
>> +/* HP CML 2020 system */
>> +{ MFG(0x4c, 0x83), PROD_ID(0x45, 0x41),
>> BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
>> /*
>> * Some Dell CML 2020 systems have panels support both AUX and PWM
>> * backlight control, and some only support AUX backlight control. All
>
> --
> Jani Nikula, Intel Open Source Graphics Center
[PATCH] ALSA: hda/hdmi: Add pins with jack detection support
HDMI on some platforms doesn't enable audio support because its Port
Connectivity [31:30] is set to AC_JACK_PORT_NONE:
Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP
Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
Amp-Out vals: [0x00 0x00]
Pincap 0x0b94: OUT Detect HBR HDMI DP
Pin Default 0x58560010: [N/A] Digital Out at Int HDMI
Conn = Digital, Color = Unknown
DefAssociation = 0x1, Sequence = 0x0
Pin-ctls: 0x40: OUT
Unsolicited: tag=00, enabled=0
Power states: D0 D3 EPSS
Power: setting=D0, actual=D0
Devices: 0
Connection: 3
0x02 0x03* 0x04
Those pins were filtered out by commit 116dcde63806 ("ALSA: HDA: Remove
unconnected PCM devices for Intel HDMI"). However, jacks that support
detection won't have the issues the commit addresses.
So still add the pin if it supports jack detection.
Signed-off-by: Kai-Heng Feng
---
sound/pci/hda/patch_hdmi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index cd46247988e4..db3a5148bd40 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1701,7 +1701,8 @@ static int hdmi_add_pin(struct hda_codec *codec,
hda_nid_t pin_nid)
* all device entries on the same pin
*/
config = snd_hda_codec_get_pincfg(codec, pin_nid);
- if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
+ if ((get_defcfg_connect(config) == AC_JACK_PORT_NONE) &&
+ !(caps & AC_PINCAP_PRES_DETECT))
return 0;
/*
--
2.17.1
Re: [PATCH] ALSA: hda/hdmi: Add pins with jack detection support
> On Aug 4, 2020, at 17:04, Takashi Iwai wrote:
>
> On Tue, 04 Aug 2020 09:29:25 +0200,
> Kai-Heng Feng wrote:
>>
>> HDMI on some platforms doesn't enable audio support because its Port
>> Connectivity [31:30] is set to AC_JACK_PORT_NONE:
>> Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP
>> Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
>> Amp-Out vals: [0x00 0x00]
>> Pincap 0x0b94: OUT Detect HBR HDMI DP
>> Pin Default 0x58560010: [N/A] Digital Out at Int HDMI
>>Conn = Digital, Color = Unknown
>>DefAssociation = 0x1, Sequence = 0x0
>> Pin-ctls: 0x40: OUT
>> Unsolicited: tag=00, enabled=0
>> Power states: D0 D3 EPSS
>> Power: setting=D0, actual=D0
>> Devices: 0
>> Connection: 3
>> 0x02 0x03* 0x04
>>
>> Those pins were filtered out by commit 116dcde63806 ("ALSA: HDA: Remove
>> unconnected PCM devices for Intel HDMI"). However, jacks that support
>> detection won't have the issues the commit addresses.
>>
>> So still add the pin if it supports jack detection.
>>
>> Signed-off-by: Kai-Heng Feng
>
> Which platform did show the problem?
An HP desktop.
>
> I'm reluctant to apply this change as it would potentially break the
> existing system. If we must to apply, maybe it's safer to apply it
> conditionally to the limited devices.
Hmm, I find it's a bit hard to match a specific device, because the ID seems to
be rather generic:
Codec: Intel Kabylake HDMI
Address: 2
AFG Function Id: 0x1 (unsol 0)
Vendor Id: 0x8086280b
Subsystem Id: 0x80860101
Revision Id: 0x10
Should we use DMI string instead?
Kai-Heng
>
>
> thanks,
>
> Takashi
>
>> ---
>> sound/pci/hda/patch_hdmi.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
>> index cd46247988e4..db3a5148bd40 100644
>> --- a/sound/pci/hda/patch_hdmi.c
>> +++ b/sound/pci/hda/patch_hdmi.c
>> @@ -1701,7 +1701,8 @@ static int hdmi_add_pin(struct hda_codec *codec,
>> hda_nid_t pin_nid)
>> * all device entries on the same pin
>> */
>> config = snd_hda_codec_get_pincfg(codec, pin_nid);
>> -if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
>> +if ((get_defcfg_connect(config) == AC_JACK_PORT_NONE) &&
>> +!(caps & AC_PINCAP_PRES_DETECT))
>> return 0;
>>
>> /*
>> --
>> 2.17.1
Re: [PATCH] ALSA: hda/hdmi: Add pins with jack detection support
> On Aug 4, 2020, at 17:48, Takashi Iwai wrote:
>
> On Tue, 04 Aug 2020 11:31:59 +0200,
> Kai-Heng Feng wrote:
>>
>>
>>
>>> On Aug 4, 2020, at 17:04, Takashi Iwai wrote:
>>>
>>> On Tue, 04 Aug 2020 09:29:25 +0200,
>>> Kai-Heng Feng wrote:
>>>>
>>>> HDMI on some platforms doesn't enable audio support because its Port
>>>> Connectivity [31:30] is set to AC_JACK_PORT_NONE:
>>>> Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP
>>>> Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
>>>> Amp-Out vals: [0x00 0x00]
>>>> Pincap 0x0b94: OUT Detect HBR HDMI DP
>>>> Pin Default 0x58560010: [N/A] Digital Out at Int HDMI
>>>> Conn = Digital, Color = Unknown
>>>> DefAssociation = 0x1, Sequence = 0x0
>>>> Pin-ctls: 0x40: OUT
>>>> Unsolicited: tag=00, enabled=0
>>>> Power states: D0 D3 EPSS
>>>> Power: setting=D0, actual=D0
>>>> Devices: 0
>>>> Connection: 3
>>>>0x02 0x03* 0x04
>>>>
>>>> Those pins were filtered out by commit 116dcde63806 ("ALSA: HDA: Remove
>>>> unconnected PCM devices for Intel HDMI"). However, jacks that support
>>>> detection won't have the issues the commit addresses.
>>>>
>>>> So still add the pin if it supports jack detection.
>>>>
>>>> Signed-off-by: Kai-Heng Feng
>>>
>>> Which platform did show the problem?
>>
>> An HP desktop.
>
> Well, what I meant was about which codec. And now I see it in the
> below.
>
>>> I'm reluctant to apply this change as it would potentially break the
>>> existing system. If we must to apply, maybe it's safer to apply it
>>> conditionally to the limited devices.
>>
>> Hmm, I find it's a bit hard to match a specific device, because the ID seems
>> to be rather generic:
>> Codec: Intel Kabylake HDMI
>> Address: 2
>> AFG Function Id: 0x1 (unsol 0)
>> Vendor Id: 0x8086280b
>> Subsystem Id: 0x80860101
>> Revision Id: 0x10
>>
>> Should we use DMI string instead?
>
> So it's a Kabylake, and I presume that it's rather an old machine.
> Is this for docking station or anything else?
The system is Comet Lake CPU so it's fairly recent. I think most Comet Lake
platforms still use "Kabylake HDMI".
>
> Basically the pin capability is rather fixed by the chip design while
> the pin configuration is set by BIOS. And we follow the BIOS setup
> for determining which pins are actually alive. That said, the bug is
> a BIOS bug.
Yes but sometimes vendors are reluctant to fix it because "Windows doesn't have
this issue".
>
> Note that PCI SSID bound with this codec might have a different
> number, so we might be still able to use the standard quirk table to
> pick up.
Ok. Will send v2 with a quirk list.
Kai-Heng
>
>
> thanks,
>
> Takashi
>
>>>
>>> thanks,
>>>
>>> Takashi
>>>
>>>> ---
>>>> sound/pci/hda/patch_hdmi.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
>>>> index cd46247988e4..db3a5148bd40 100644
>>>> --- a/sound/pci/hda/patch_hdmi.c
>>>> +++ b/sound/pci/hda/patch_hdmi.c
>>>> @@ -1701,7 +1701,8 @@ static int hdmi_add_pin(struct hda_codec *codec,
>>>> hda_nid_t pin_nid)
>>>> * all device entries on the same pin
>>>> */
>>>>config = snd_hda_codec_get_pincfg(codec, pin_nid);
>>>> - if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
>>>> + if ((get_defcfg_connect(config) == AC_JACK_PORT_NONE) &&
>>>> + !(caps & AC_PINCAP_PRES_DETECT))
>>>>return 0;
>>>>
>>>>/*
>>>> --
>>>> 2.17.1
[PATCH v2] ALSA: hda/hdmi: Add quirk to force connectivity
HDMI on some platforms doesn't enable audio support because its Port
Connectivity [31:30] is set to AC_JACK_PORT_NONE:
Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP
Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
Amp-Out vals: [0x00 0x00]
Pincap 0x0b94: OUT Detect HBR HDMI DP
Pin Default 0x58560010: [N/A] Digital Out at Int HDMI
Conn = Digital, Color = Unknown
DefAssociation = 0x1, Sequence = 0x0
Pin-ctls: 0x40: OUT
Unsolicited: tag=00, enabled=0
Power states: D0 D3 EPSS
Power: setting=D0, actual=D0
Devices: 0
Connection: 3
0x02 0x03* 0x04
For now, use a quirk to force connectivity based on SSID. If there are
more platforms affected by the same issue, we can eye for a more generic
solution.
Signed-off-by: Kai-Heng Feng
---
v2:
- Use a quirk list instead.
include/sound/hda_codec.h | 1 +
sound/pci/hda/patch_hdmi.c | 14 +-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index d16a4229209b..947cba68d9fe 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -254,6 +254,7 @@ struct hda_codec {
unsigned int link_down_at_suspend:1; /* link down at runtime suspend */
unsigned int relaxed_resume:1; /* don't resume forcibly for jack */
unsigned int mst_no_extra_pcms:1; /* no backup PCMs for DP-MST */
+ unsigned int force_connect:1; /* force connectivity */
#ifdef CONFIG_PM
unsigned long power_on_acct;
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index cd46247988e4..40d4f0e0e3d7 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1701,7 +1701,8 @@ static int hdmi_add_pin(struct hda_codec *codec,
hda_nid_t pin_nid)
* all device entries on the same pin
*/
config = snd_hda_codec_get_pincfg(codec, pin_nid);
- if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
+ if (get_defcfg_connect(config) == AC_JACK_PORT_NONE &&
+ !codec->force_connect)
return 0;
/*
@@ -1803,11 +1804,17 @@ static int hdmi_add_cvt(struct hda_codec *codec,
hda_nid_t cvt_nid)
return 0;
}
+static const struct snd_pci_quirk force_connect_list[] = {
+ SND_PCI_QUIRK(0x103c, 0x871a, "HP", 1),
+ {}
+};
+
static int hdmi_parse_codec(struct hda_codec *codec)
{
hda_nid_t start_nid;
unsigned int caps;
int i, nodes;
+ const struct snd_pci_quirk *q;
nodes = snd_hda_get_sub_nodes(codec, codec->core.afg, _nid);
if (!start_nid || nodes < 0) {
@@ -1815,6 +1822,11 @@ static int hdmi_parse_codec(struct hda_codec *codec)
return -EINVAL;
}
+ q = snd_pci_quirk_lookup(codec->bus->pci, force_connect_list);
+
+ if (q && q->value)
+ codec->force_connect = 1;
+
/*
* hdmi_add_pin() assumes total amount of converters to
* be known, so first discover all converters
--
2.17.1
[PATCH v3] ALSA: hda/hdmi: Add quirk to force connectivity
HDMI on some platforms doesn't enable audio support because its Port
Connectivity [31:30] is set to AC_JACK_PORT_NONE:
Node 0x05 [Pin Complex] wcaps 0x40778d: 8-Channels Digital Amp-Out CP
Amp-Out caps: ofs=0x00, nsteps=0x00, stepsize=0x00, mute=1
Amp-Out vals: [0x00 0x00]
Pincap 0x0b94: OUT Detect HBR HDMI DP
Pin Default 0x58560010: [N/A] Digital Out at Int HDMI
Conn = Digital, Color = Unknown
DefAssociation = 0x1, Sequence = 0x0
Pin-ctls: 0x40: OUT
Unsolicited: tag=00, enabled=0
Power states: D0 D3 EPSS
Power: setting=D0, actual=D0
Devices: 0
Connection: 3
0x02 0x03* 0x04
For now, use a quirk to force connectivity based on SSID. If there are
more platforms affected by the same issue, we can eye for a more generic
solution.
Signed-off-by: Kai-Heng Feng
---
v3:
- Move the flag into hdmi_spec.
v2:
- Use a quirk list instead.
sound/pci/hda/patch_hdmi.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index cd46247988e4..b62cd3abb827 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -160,6 +160,7 @@ struct hdmi_spec {
bool use_acomp_notifier; /* use eld_notify callback for hotplug */
bool acomp_registered; /* audio component registered in this driver */
+ bool force_connect; /* force connectivity */
struct drm_audio_component_audio_ops drm_audio_ops;
int (*port2pin)(struct hda_codec *, int); /* reverse port/pin mapping */
@@ -1701,7 +1702,8 @@ static int hdmi_add_pin(struct hda_codec *codec,
hda_nid_t pin_nid)
* all device entries on the same pin
*/
config = snd_hda_codec_get_pincfg(codec, pin_nid);
- if (get_defcfg_connect(config) == AC_JACK_PORT_NONE)
+ if (get_defcfg_connect(config) == AC_JACK_PORT_NONE &&
+ !spec->force_connect)
return 0;
/*
@@ -1803,11 +1805,18 @@ static int hdmi_add_cvt(struct hda_codec *codec,
hda_nid_t cvt_nid)
return 0;
}
+static const struct snd_pci_quirk force_connect_list[] = {
+ SND_PCI_QUIRK(0x103c, 0x871a, "HP", 1),
+ {}
+};
+
static int hdmi_parse_codec(struct hda_codec *codec)
{
+ struct hdmi_spec *spec = codec->spec;
hda_nid_t start_nid;
unsigned int caps;
int i, nodes;
+ const struct snd_pci_quirk *q;
nodes = snd_hda_get_sub_nodes(codec, codec->core.afg, _nid);
if (!start_nid || nodes < 0) {
@@ -1815,6 +1824,11 @@ static int hdmi_parse_codec(struct hda_codec *codec)
return -EINVAL;
}
+ q = snd_pci_quirk_lookup(codec->bus->pci, force_connect_list);
+
+ if (q && q->value)
+ spec->force_connect = true;
+
/*
* hdmi_add_pin() assumes total amount of converters to
* be known, so first discover all converters
--
2.17.1
[PATCH] rtw88: 8821c: Add RFE 2 support
8821CE with RFE 2 isn't supported:
[ 12.404834] rtw_8821ce :02:00.0: rfe 2 isn't supported
[ 12.404937] rtw_8821ce :02:00.0: failed to setup chip efuse info
[ 12.404939] rtw_8821ce :02:00.0: failed to setup chip information
It works well if both type0 tables are in use, so add it to the RFE
default.
Signed-off-by: Kai-Heng Feng
---
drivers/net/wireless/realtek/rtw88/rtw8821c.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c
b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
index d8863d8a5468..f7270d0f1d55 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
@@ -1410,6 +1410,7 @@ static const struct rtw_intf_phy_para_table
phy_para_table_8821c = {
static const struct rtw_rfe_def rtw8821c_rfe_defs[] = {
[0] = RTW_DEF_RFE(8821c, 0, 0),
+ [2] = RTW_DEF_RFE(8821c, 0, 0),
};
static struct rtw_hw_reg rtw8821c_dig[] = {
--
2.17.1
Re: [PATCH] rtw88: 8821c: Add RFE 2 support
Hi Tony, > On Aug 5, 2020, at 19:18, Tony Chuang wrote: > >> 8821CE with RFE 2 isn't supported: >> [ 12.404834] rtw_8821ce :02:00.0: rfe 2 isn't supported >> [ 12.404937] rtw_8821ce :02:00.0: failed to setup chip efuse info >> [ 12.404939] rtw_8821ce :02:00.0: failed to setup chip information >> > > NACK > > The RFE type 2 should be working with some additional fixes. > Did you tested connecting to AP with BT paired? No, I only tested WiFi. > The antenna configuration is different with RFE type 0. > I will ask someone else to fix them. > Then the RFE type 2 modules can be supported. Good to know that, I'll be patient and wait for a real fix. Kai-Heng > > Yen-Hsuan
Re: [Intel-wired-lan] [PATCH v4] e1000e: Increase polling timeout on MDIC ready bit
Hi Vitaly,
> On Sep 30, 2020, at 14:54, Vitaly Lifshits wrote:
>
> On 9/29/2020 18:08, Kai-Heng Feng wrote:
>
> Hello Kai-Heng,
>>> On Sep 29, 2020, at 21:46, Neftin, Sasha wrote:
>>>
>>> Hello Kai-Heng,
>>> On 9/29/2020 16:31, Kai-Heng Feng wrote:
>>>> Hi Sasha,
>>>>> On Sep 29, 2020, at 21:08, Neftin, Sasha wrote:
>>>>>
>>>>> On 9/28/2020 11:36, Kai-Heng Feng wrote:
>>>>>> We are seeing the following error after S3 resume:
>>>>>> [ 704.746874] e1000e :00:1f.6 eno1: Setting page 0x6020
>>>>>> [ 704.844232] e1000e :00:1f.6 eno1: MDI Write did not complete
>>>>>> [ 704.902817] e1000e :00:1f.6 eno1: Setting page 0x6020
>>>>>> [ 704.903075] e1000e :00:1f.6 eno1: reading PHY page 769 (or 0x6020
>>>>>> shifted) reg 0x17
>>>>>> [ 704.903281] e1000e :00:1f.6 eno1: Setting page 0x6020
>>>>>> [ 704.903486] e1000e :00:1f.6 eno1: writing PHY page 769 (or 0x6020
>>>>>> shifted) reg 0x17
>>>>>> [ 704.943155] e1000e :00:1f.6 eno1: MDI Error
>>>>>> ...
>>>>>> [ 705.108161] e1000e :00:1f.6 eno1: Hardware Error
>>>>>> As Andrew Lunn pointed out, MDIO has nothing to do with phy, and indeed
>>>>>> increase polling iteration can resolve the issue.
>>>>>> This patch only papers over the symptom, as we don't really know the
>>>>>> root cause of the issue. The most possible culprit is Intel ME, which
>>>>>> may do its own things that conflict with software.
>>>>>> Signed-off-by: Kai-Heng Feng
>>>>>> ---
>>>>>> v4:
>>>>>> - States that this patch just papers over the symptom.
>>>>>> v3:
>>>>>> - Moving delay to end of loop doesn't save anytime, move it back.
>>>>>> - Point out this is quitely likely caused by Intel ME.
>>>>>> v2:
>>>>>> - Increase polling iteration instead of powering down the phy.
>>>>>> drivers/net/ethernet/intel/e1000e/phy.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>> diff --git a/drivers/net/ethernet/intel/e1000e/phy.c
>>>>>> b/drivers/net/ethernet/intel/e1000e/phy.c
>>>>>> index e11c877595fb..e6d4acd90937 100644
>>>>>> --- a/drivers/net/ethernet/intel/e1000e/phy.c
>>>>>> +++ b/drivers/net/ethernet/intel/e1000e/phy.c
>>>>>> @@ -203,7 +203,7 @@ s32 e1000e_write_phy_reg_mdic(struct e1000_hw *hw,
>>>>>> u32 offset, u16 data)
>>>>>> * Increasing the time out as testing showed failures with
>>>>>> * the lower time out
>>>>>> */
>>>>>> -for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
>>>>>> +for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 10); i++) {
>>>>> As we discussed (many threads) - AMT/ME systems not supported on Linux as
>>>>> properly. I do not think increasing polling iteration will solve the
>>>>> problem. Rather mask it.
>>>> I am aware of the status quo of no proper support on Intel ME.
>>>>> I prefer you check option to disable ME vi BIOS on your system.
>>>> We can't ask user to change the BIOS to accommodate Linux. So before a
>>>> proper solution comes out, masking the problem is good enough for me.
>>>> Until then, I'll carry it as a downstream distro patch.
>>> What will you do with system that even after increasing polling time will
>>> run into HW error?
>> Hope we finally have proper ME support under Linux?
>> Kai-Heng
>>>> Kai-Heng
>>>>>> udelay(50);
>>>>>> mdic = er32(MDIC);
>>>>>> if (mdic & E1000_MDIC_READY)
>>>>> Thanks,
>>>>> Sasha
>>> Thanks,
>>> Sasha
>
> On which device ID/platform do you see the issue?
HP Z4 G4 Workstation,
00:1f.6 Ethernet controller [0200]: Intel Corporation Ethernet Connection (2)
I219-LM [8086:15b7]
> What is the Firmware version on your platform?
BIOS version: P61 v02.59
> What is the ME firmware version that you have?
ME version: 11.11.50.1422
ME mode: AMT disable
Kai-Heng
>
> I am asking these questions, since I know there is supposed to be a fix in
> the firmware to many issues that are related to ME and device
> interoperability.
>
> Thanks,
>
> Vitaly
[PATCH] ALSA: usb-audio: Disable Lenovo P620 Rear line-in volume control
The USB device (0x17aa:0x1046) that support Lenovo P620 rear panel
line-in claim to support volume control, but it doens't seem to have an
AMP, so when line-in volume lowers below 80, nothing gets recorded
anymore.
Disable the volume control to workaround the issue.
Signed-off-by: Kai-Heng Feng
---
sound/usb/mixer_maps.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/usb/mixer_maps.c b/sound/usb/mixer_maps.c
index c369c81e74c4..5b43e9e40e49 100644
--- a/sound/usb/mixer_maps.c
+++ b/sound/usb/mixer_maps.c
@@ -371,6 +371,7 @@ static const struct usbmix_name_map asus_rog_map[] = {
};
static const struct usbmix_name_map lenovo_p620_rear_map[] = {
+ { 19, NULL, 2 }, /* FU, Volume */
{ 19, NULL, 12 }, /* FU, Input Gain Pad */
{}
};
--
2.17.1
[PATCH] xhci: Do warm-reset when both CAS and XDEV_RESUME are set
Sometimes re-plugging a USB device during system sleep renders the device
useless:
[ 173.418345] xhci_hcd :00:14.0: Get port status 2-4 read: 0x14203e2,
return 0x10262
...
[ 176.496485] usb 2-4: Waited 2000ms for CONNECT
[ 176.496781] usb usb2-port4: status .0262 after resume, -19
[ 176.497103] usb 2-4: can't resume, status -19
[ 176.497438] usb usb2-port4: logical disconnect
Because PLS equals to XDEV_RESUME, xHCI driver reports U3 to usbcore,
despite of CAS bit is flagged.
So proritize CAS over XDEV_RESUME to let usbcore handle warm-reset for
the port.
Signed-off-by: Kai-Heng Feng
---
drivers/usb/host/xhci-hub.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index c3554e37e09f..4e14e164cb68 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -740,15 +740,6 @@ static void xhci_hub_report_usb3_link_state(struct
xhci_hcd *xhci,
{
u32 pls = status_reg & PORT_PLS_MASK;
- /* resume state is a xHCI internal state.
-* Do not report it to usb core, instead, pretend to be U3,
-* thus usb core knows it's not ready for transfer
-*/
- if (pls == XDEV_RESUME) {
- *status |= USB_SS_PORT_LS_U3;
- return;
- }
-
/* When the CAS bit is set then warm reset
* should be performed on port
*/
@@ -770,6 +761,16 @@ static void xhci_hub_report_usb3_link_state(struct
xhci_hcd *xhci,
*/
pls |= USB_PORT_STAT_CONNECTION;
} else {
+ /*
+* Resume state is an xHCI internal state. Do not report it to
+* usb core, instead, pretend to be U3, thus usb core knows
+* it's not ready for transfer.
+*/
+ if (pls == XDEV_RESUME) {
+ *status |= USB_SS_PORT_LS_U3;
+ return;
+ }
+
/*
* If CAS bit isn't set but the Port is already at
* Compliance Mode, fake a connection so the USB core
--
2.17.1
[PATCH] HID: i2c-hid: Add 60ms delay after SET_POWER ON
Goodix touchpad fails to operate in I2C mode after system suspend. According to the vendor, Windows is more forgiving and there's a 60ms delay after SET_POWER ON command. So let's do the same here, to workaround for the touchpads that depend on the delay. Signed-off-by: Kai-Heng Feng --- drivers/hid/i2c-hid/i2c-hid-core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index 294c84e136d7..7b24a27fad95 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -419,6 +419,9 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state) if (ret) dev_err(>dev, "failed to change power setting.\n"); + if (!ret && power_state == I2C_HID_PWR_ON) + msleep(60); + set_pwr_exit: return ret; } -- 2.17.1
Re: [PATCH] HID: i2c-hid: Add 60ms delay after SET_POWER ON
Hi Hans, > On Aug 11, 2020, at 00:13, Hans de Goede wrote: > > Hi, > > On 10-08-2020 16:29, Kai-Heng Feng wrote: >> Goodix touchpad fails to operate in I2C mode after system suspend. >> According to the vendor, Windows is more forgiving and there's a 60ms >> delay after SET_POWER ON command. >> So let's do the same here, to workaround for the touchpads that depend >> on the delay. >> Signed-off-by: Kai-Heng Feng > > Interesting I send a very similar patch a couple of days ago, > after debugging some touchpads issues on a BMAX Y13 laptop: > > https://patchwork.kernel.org/patch/11701541/ > > If you look at that patch you will see that if we add a > sleep on power-on to i2c_hid_set_power(), we can remove > an existing sleep after power-on from i2c_hid_hwreset(). > > And there is an interesting comment there which should > probably be moved (as my patch does) and corrected for the > new knowledge so that people reading the code in the future > now why the sleep is there. Thanks for the info. Can you please update your patch with 60ms to supersede mine? > > Other then that we've come to the same conclusion, but > your sleep is much longer. I guess that is ok though, > are you sure we need 60ms as a minimum? > Is that what goodix > said? Yes, I was told by Goodix that the 60ms delay is needed. Kai-Heng > > Regards, > > Hans > > >> --- >> drivers/hid/i2c-hid/i2c-hid-core.c | 3 +++ >> 1 file changed, 3 insertions(+) >> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c >> b/drivers/hid/i2c-hid/i2c-hid-core.c >> index 294c84e136d7..7b24a27fad95 100644 >> --- a/drivers/hid/i2c-hid/i2c-hid-core.c >> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c >> @@ -419,6 +419,9 @@ static int i2c_hid_set_power(struct i2c_client *client, >> int power_state) >> if (ret) >> dev_err(>dev, "failed to change power setting.\n"); >> + if (!ret && power_state == I2C_HID_PWR_ON) >> +msleep(60); >> + >> set_pwr_exit: >> return ret; >> } >
[PATCH] ALSA: hda/hdmi: Use force connectivity quirk on another HP desktop
There's another HP desktop has buggy BIOS which flags the Port
Connectivity bit as no connection.
Apply force connectivity quirk to enable DP/HDMI audio.
Signed-off-by: Kai-Heng Feng
---
sound/pci/hda/patch_hdmi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 4bbd12d3b1b5..b8c8490e568b 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1863,6 +1863,7 @@ static int hdmi_add_cvt(struct hda_codec *codec,
hda_nid_t cvt_nid)
}
static const struct snd_pci_quirk force_connect_list[] = {
+ SND_PCI_QUIRK(0x103c, 0x870f, "HP", 1),
SND_PCI_QUIRK(0x103c, 0x871a, "HP", 1),
{}
};
--
2.17.1
Re: [Intel-wired-lan] [PATCH v4] e1000e: Increase polling timeout on MDIC ready bit
Hi Sasha,
> On Sep 29, 2020, at 21:08, Neftin, Sasha wrote:
>
> On 9/28/2020 11:36, Kai-Heng Feng wrote:
>> We are seeing the following error after S3 resume:
>> [ 704.746874] e1000e :00:1f.6 eno1: Setting page 0x6020
>> [ 704.844232] e1000e :00:1f.6 eno1: MDI Write did not complete
>> [ 704.902817] e1000e :00:1f.6 eno1: Setting page 0x6020
>> [ 704.903075] e1000e :00:1f.6 eno1: reading PHY page 769 (or 0x6020
>> shifted) reg 0x17
>> [ 704.903281] e1000e :00:1f.6 eno1: Setting page 0x6020
>> [ 704.903486] e1000e :00:1f.6 eno1: writing PHY page 769 (or 0x6020
>> shifted) reg 0x17
>> [ 704.943155] e1000e :00:1f.6 eno1: MDI Error
>> ...
>> [ 705.108161] e1000e :00:1f.6 eno1: Hardware Error
>> As Andrew Lunn pointed out, MDIO has nothing to do with phy, and indeed
>> increase polling iteration can resolve the issue.
>> This patch only papers over the symptom, as we don't really know the
>> root cause of the issue. The most possible culprit is Intel ME, which
>> may do its own things that conflict with software.
>> Signed-off-by: Kai-Heng Feng
>> ---
>> v4:
>> - States that this patch just papers over the symptom.
>> v3:
>> - Moving delay to end of loop doesn't save anytime, move it back.
>> - Point out this is quitely likely caused by Intel ME.
>> v2:
>> - Increase polling iteration instead of powering down the phy.
>> drivers/net/ethernet/intel/e1000e/phy.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/drivers/net/ethernet/intel/e1000e/phy.c
>> b/drivers/net/ethernet/intel/e1000e/phy.c
>> index e11c877595fb..e6d4acd90937 100644
>> --- a/drivers/net/ethernet/intel/e1000e/phy.c
>> +++ b/drivers/net/ethernet/intel/e1000e/phy.c
>> @@ -203,7 +203,7 @@ s32 e1000e_write_phy_reg_mdic(struct e1000_hw *hw, u32
>> offset, u16 data)
>> * Increasing the time out as testing showed failures with
>> * the lower time out
>> */
>> -for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
>> +for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 10); i++) {
> As we discussed (many threads) - AMT/ME systems not supported on Linux as
> properly. I do not think increasing polling iteration will solve the problem.
> Rather mask it.
I am aware of the status quo of no proper support on Intel ME.
> I prefer you check option to disable ME vi BIOS on your system.
We can't ask user to change the BIOS to accommodate Linux. So before a proper
solution comes out, masking the problem is good enough for me.
Until then, I'll carry it as a downstream distro patch.
Kai-Heng
>> udelay(50);
>> mdic = er32(MDIC);
>> if (mdic & E1000_MDIC_READY)
> Thanks,
> Sasha
Re: [Intel-wired-lan] [PATCH v4] e1000e: Increase polling timeout on MDIC ready bit
> On Sep 29, 2020, at 21:46, Neftin, Sasha wrote:
>
> Hello Kai-Heng,
> On 9/29/2020 16:31, Kai-Heng Feng wrote:
>> Hi Sasha,
>>> On Sep 29, 2020, at 21:08, Neftin, Sasha wrote:
>>>
>>> On 9/28/2020 11:36, Kai-Heng Feng wrote:
>>>> We are seeing the following error after S3 resume:
>>>> [ 704.746874] e1000e :00:1f.6 eno1: Setting page 0x6020
>>>> [ 704.844232] e1000e :00:1f.6 eno1: MDI Write did not complete
>>>> [ 704.902817] e1000e :00:1f.6 eno1: Setting page 0x6020
>>>> [ 704.903075] e1000e :00:1f.6 eno1: reading PHY page 769 (or 0x6020
>>>> shifted) reg 0x17
>>>> [ 704.903281] e1000e :00:1f.6 eno1: Setting page 0x6020
>>>> [ 704.903486] e1000e :00:1f.6 eno1: writing PHY page 769 (or 0x6020
>>>> shifted) reg 0x17
>>>> [ 704.943155] e1000e :00:1f.6 eno1: MDI Error
>>>> ...
>>>> [ 705.108161] e1000e :00:1f.6 eno1: Hardware Error
>>>> As Andrew Lunn pointed out, MDIO has nothing to do with phy, and indeed
>>>> increase polling iteration can resolve the issue.
>>>> This patch only papers over the symptom, as we don't really know the
>>>> root cause of the issue. The most possible culprit is Intel ME, which
>>>> may do its own things that conflict with software.
>>>> Signed-off-by: Kai-Heng Feng
>>>> ---
>>>> v4:
>>>> - States that this patch just papers over the symptom.
>>>> v3:
>>>> - Moving delay to end of loop doesn't save anytime, move it back.
>>>> - Point out this is quitely likely caused by Intel ME.
>>>> v2:
>>>> - Increase polling iteration instead of powering down the phy.
>>>> drivers/net/ethernet/intel/e1000e/phy.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> diff --git a/drivers/net/ethernet/intel/e1000e/phy.c
>>>> b/drivers/net/ethernet/intel/e1000e/phy.c
>>>> index e11c877595fb..e6d4acd90937 100644
>>>> --- a/drivers/net/ethernet/intel/e1000e/phy.c
>>>> +++ b/drivers/net/ethernet/intel/e1000e/phy.c
>>>> @@ -203,7 +203,7 @@ s32 e1000e_write_phy_reg_mdic(struct e1000_hw *hw, u32
>>>> offset, u16 data)
>>>> * Increasing the time out as testing showed failures with
>>>> * the lower time out
>>>> */
>>>> - for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 3); i++) {
>>>> + for (i = 0; i < (E1000_GEN_POLL_TIMEOUT * 10); i++) {
>>> As we discussed (many threads) - AMT/ME systems not supported on Linux as
>>> properly. I do not think increasing polling iteration will solve the
>>> problem. Rather mask it.
>> I am aware of the status quo of no proper support on Intel ME.
>>> I prefer you check option to disable ME vi BIOS on your system.
>> We can't ask user to change the BIOS to accommodate Linux. So before a
>> proper solution comes out, masking the problem is good enough for me.
>> Until then, I'll carry it as a downstream distro patch.
> What will you do with system that even after increasing polling time will run
> into HW error?
Hope we finally have proper ME support under Linux?
Kai-Heng
>> Kai-Heng
>>>>udelay(50);
>>>>mdic = er32(MDIC);
>>>>if (mdic & E1000_MDIC_READY)
>>> Thanks,
>>> Sasha
> Thanks,
> Sasha
Re: [Intel-wired-lan] [PATCH v4] e1000e: Increase polling timeout on MDIC ready bit
> On Sep 29, 2020, at 23:11, David Laight wrote: > >> Hope we finally have proper ME support under Linux? > > How about a way to disable it. This will do, too :) Kai-Heng > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 > 1PT, UK > Registration No: 1397386 (Wales) >
[PATCH 1/2] PCI/ASPM: Add helper to enable ASPM link
Platform firmware may not be able to access config space to program
ASPM. For instance, devices behind Intel VMD are not configured by the
BIOS.
So add a helper to let drivers have an option to enable ASPM.
Signed-off-by: Kai-Heng Feng
---
drivers/pci/pcie/aspm.c | 73 ++---
include/linux/pci.h | 7
2 files changed, 60 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 253c30cc1967..b5ac6a99e016 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1080,6 +1080,58 @@ static struct pcie_link_state *pcie_aspm_get_link(struct
pci_dev *pdev)
return bridge->link_state;
}
+static u32 pcie_aspm_convert_state(int state)
+{
+ u32 aspm = 0;
+
+ if (state & PCIE_LINK_STATE_L0S)
+ aspm |= ASPM_STATE_L0S;
+ if (state & PCIE_LINK_STATE_L1)
+ /* L1 PM substates require L1 */
+ aspm |= ASPM_STATE_L1 | ASPM_STATE_L1SS;
+ if (state & PCIE_LINK_STATE_L1_1)
+ aspm |= ASPM_STATE_L1_1;
+ if (state & PCIE_LINK_STATE_L1_2)
+ aspm |= ASPM_STATE_L1_2;
+ if (state & PCIE_LINK_STATE_L1_1_PCIPM)
+ aspm |= ASPM_STATE_L1_1_PCIPM;
+ if (state & PCIE_LINK_STATE_L1_2_PCIPM)
+ aspm |= ASPM_STATE_L1_2_PCIPM;
+
+ return aspm;
+}
+
+static void pci_set_link_state(struct pci_dev *pdev, bool enable, int state)
+{
+ struct pcie_link_state *link = pcie_aspm_get_link(pdev);
+
+ mutex_lock(_lock);
+ if (enable)
+ link->aspm_default = pcie_aspm_convert_state(state);
+ else
+ link->aspm_disable = pcie_aspm_convert_state(state);
+ pcie_config_aspm_link(link, policy_to_aspm_state(link));
+
+ if (state & PCIE_LINK_STATE_CLKPM) {
+ link->clkpm_default = enable;
+ link->clkpm_disable = !enable;
+ }
+ pcie_set_clkpm(link, policy_to_clkpm_state(link));
+ mutex_unlock(_lock);
+}
+
+int pci_enable_link_state(struct pci_dev *pdev, int state)
+{
+ struct pcie_link_state *link = pcie_aspm_get_link(pdev);
+
+ if (!link)
+ return -EINVAL;
+
+ pci_set_link_state(pdev, true, state);
+ return 0;
+}
+EXPORT_SYMBOL(pci_enable_link_state);
+
static int __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
{
struct pcie_link_state *link = pcie_aspm_get_link(pdev);
@@ -1101,26 +1153,7 @@ static int __pci_disable_link_state(struct pci_dev
*pdev, int state, bool sem)
if (sem)
down_read(_bus_sem);
- mutex_lock(_lock);
- if (state & PCIE_LINK_STATE_L0S)
- link->aspm_disable |= ASPM_STATE_L0S;
- if (state & PCIE_LINK_STATE_L1)
- /* L1 PM substates require L1 */
- link->aspm_disable |= ASPM_STATE_L1 | ASPM_STATE_L1SS;
- if (state & PCIE_LINK_STATE_L1_1)
- link->aspm_disable |= ASPM_STATE_L1_1;
- if (state & PCIE_LINK_STATE_L1_2)
- link->aspm_disable |= ASPM_STATE_L1_2;
- if (state & PCIE_LINK_STATE_L1_1_PCIPM)
- link->aspm_disable |= ASPM_STATE_L1_1_PCIPM;
- if (state & PCIE_LINK_STATE_L1_2_PCIPM)
- link->aspm_disable |= ASPM_STATE_L1_2_PCIPM;
- pcie_config_aspm_link(link, policy_to_aspm_state(link));
-
- if (state & PCIE_LINK_STATE_CLKPM)
- link->clkpm_disable = 1;
- pcie_set_clkpm(link, policy_to_clkpm_state(link));
- mutex_unlock(_lock);
+ pci_set_link_state(pdev, false, state);
if (sem)
up_read(_bus_sem);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 835530605c0d..a6a7830ec258 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1568,14 +1568,21 @@ extern bool pcie_ports_native;
#define PCIE_LINK_STATE_L1_2 BIT(4)
#define PCIE_LINK_STATE_L1_1_PCIPM BIT(5)
#define PCIE_LINK_STATE_L1_2_PCIPM BIT(6)
+#define PCIE_LINK_STATE_ALL(PCIE_LINK_STATE_L0S |
PCIE_LINK_STATE_L1 |\
+PCIE_LINK_STATE_CLKPM |
PCIE_LINK_STATE_L1_1 |\
+PCIE_LINK_STATE_L1_2 |
PCIE_LINK_STATE_L1_1_PCIPM |\
+PCIE_LINK_STATE_L1_2_PCIPM)
#ifdef CONFIG_PCIEASPM
+int pci_enable_link_state(struct pci_dev *pdev, int state);
int pci_disable_link_state(struct pci_dev *pdev, int state);
int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
void pcie_no_aspm(void);
bool pcie_aspm_support_enabled(void);
bool pcie_aspm_enabled(struct pci_dev *pdev);
#else
+static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
+{ return 0; }
static inline int pci_disable_link_state(struct pci_dev *pdev, int state)
{ return 0; }
static inline int pci_disable_link_state_locked(struct pci_dev *pdev, int
state)
--
2.17.1
[PATCH 2/2] PCI: vmd: Enable ASPM for mobile platforms
BIOS may not be able to program ASPM for links behind VMD, prevent Intel
SoC from entering deeper power saving state.
So enable ASPM for links behind VMD to increase battery life.
Signed-off-by: Kai-Heng Feng
---
drivers/pci/controller/vmd.c | 22 +-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index f69ef8c89f72..058fdef9c566 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -417,6 +417,22 @@ static int vmd_find_free_domain(void)
return domain + 1;
}
+static const struct pci_device_id vmd_mobile_bridge_tbl[] = {
+ { PCI_VDEVICE(INTEL, 0x9a09) },
+ { PCI_VDEVICE(INTEL, 0xa0b0) },
+ { PCI_VDEVICE(INTEL, 0xa0bc) },
+ { }
+};
+
+static int vmd_enable_aspm(struct device *dev, void *data)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL);
+
+ return 0;
+}
+
static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
{
struct pci_sysdata *sd = >sysdata;
@@ -603,8 +619,12 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned
long features)
* and will fail pcie_bus_configure_settings() early. It can instead be
* run on each of the real root ports.
*/
- list_for_each_entry(child, >bus->children, node)
+ list_for_each_entry(child, >bus->children, node) {
+ if (pci_match_id(vmd_mobile_bridge_tbl, child->self))
+ device_for_each_child(>self->dev, NULL,
vmd_enable_aspm);
+
pcie_bus_configure_settings(child);
+ }
pci_bus_add_devices(vmd->bus);
--
2.17.1
Re: [PATCH] nvme-pci: Disable Write Zeroes on Sandisk Skyhawk
> On Oct 14, 2020, at 08:20, Chaitanya Kulkarni
> wrote:
>
> On 10/13/20 01:45, Kai-Heng Feng wrote:
>> Like commit 5611ec2b9814 ("nvme-pci: prevent SK hynix PC400 from using
>> Write Zeroes command"), Sandisk Skyhawk has the same issue:
>> [ 6305.633887] blk_update_request: operation not supported error, dev
>> nvme0n1, sector 340812032 op 0x9:(WRITE_ZEROES) flags 0x0 phys_seg 0 prio
>> class 0
>>
>> So also disable Write Zeroes command on Sandisk Skyhawk.
>>
>> BugLink: https://bugs.launchpad.net/bugs/1899503
>> Signed-off-by: Kai-Heng Feng
>
> Are you sure this happens all the devices of the same model
>
> and not a firmware bug on specific device ?
>
> If yes then looks good.
Yes, and this is the reply from WD, requested by the user:
"These are newest WD branded drives and WD already confirmed they are
the newest firmware / no upgrades are available."
Kai-Heng
>
> Reviewed-by: Chaitanya Kulkarni
>
>
>
Re: [PATCH 1/2] drm/i915/dpcd_bl: Skip testing control capability with force DPCD quirk
Hi Lyude,
> On Oct 8, 2020, at 05:53, Lyude Paul wrote:
>
> Hi! I thought this patch rang a bell, we actually already had some discussion
> about this since there's a couple of other systems this was causing issues
> for.
> Unfortunately it never seems like that patch got sent out. Satadru?
>
> (if I don't hear back from them soon, I'll just send out a patch for this
> myself)
>
> JFYI - the proper fix here is to just drop the
> DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP check from the code entirely. As long
> as
> the backlight supports AUX_SET_CAP, that should be enough for us to control
> it.
Does the proper fix include dropping DP_QUIRK_FORCE_DPCD_BACKLIGHT entirely?
Kai-Heng
>
>
> On Wed, 2020-10-07 at 14:58 +0800, Kai-Heng Feng wrote:
>> HP DreamColor panel needs to be controlled via AUX interface. However,
>> it has both DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP and
>> DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP set, so it fails to pass
>> intel_dp_aux_display_control_capable() test.
>>
>> Skip the test if the panel has force DPCD quirk.
>>
>> Signed-off-by: Kai-Heng Feng
>> ---
>> drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 10 ++
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> index acbd7eb66cbe..acf2e1c65290 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
>> @@ -347,9 +347,13 @@ int intel_dp_aux_init_backlight_funcs(struct
>> intel_connector *intel_connector)
>> struct intel_panel *panel = _connector->panel;
>> struct intel_dp *intel_dp = enc_to_intel_dp(intel_connector->encoder);
>> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>> +bool force_dpcd;
>> +
>> +force_dpcd = drm_dp_has_quirk(_dp->desc, intel_dp->edid_quirks,
>> + DP_QUIRK_FORCE_DPCD_BACKLIGHT);
>>
>> if (i915->params.enable_dpcd_backlight == 0 ||
>> -!intel_dp_aux_display_control_capable(intel_connector))
>> +(!force_dpcd &&
>> !intel_dp_aux_display_control_capable(intel_connector)))
>> return -ENODEV;
>>
>> /*
>> @@ -358,9 +362,7 @@ int intel_dp_aux_init_backlight_funcs(struct
>> intel_connector *intel_connector)
>> */
>> if (i915->vbt.backlight.type !=
>> INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE &&
>> -i915->params.enable_dpcd_backlight != 1 &&
>> -!drm_dp_has_quirk(_dp->desc, intel_dp->edid_quirks,
>> - DP_QUIRK_FORCE_DPCD_BACKLIGHT)) {
>> +i915->params.enable_dpcd_backlight != 1 && !force_dpcd) {
>> drm_info(>drm,
>> "Panel advertises DPCD backlight support, but "
>> "VBT disagrees. If your backlight controls "
> --
> Sincerely,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
Re: [PATCH 2/2] PCI: vmd: Enable ASPM for mobile platforms
> On Oct 7, 2020, at 21:30, Bjorn Helgaas wrote:
>
> On Wed, Oct 07, 2020 at 12:26:19PM +0800, Kai-Heng Feng wrote:
>>> On Oct 6, 2020, at 03:19, Bjorn Helgaas wrote:
>>> On Tue, Oct 06, 2020 at 02:40:32AM +0800, Kai-Heng Feng wrote:
>>>>> On Oct 3, 2020, at 06:18, Bjorn Helgaas wrote:
>>>>> On Wed, Sep 30, 2020 at 04:24:54PM +0800, Kai-Heng Feng wrote:
>
> ...
>>>> I wonder whether other devices that add PCIe domain have the same
>>>> behavior? Maybe it's not a special case at all...
>>>
>>> What other devices are these?
>>
>> Controllers which add PCIe domain.
>
> I was looking for specific examples, not just a restatement of what
> you said before. I'm just curious because there are a lot of
> controllers I'm not familiar with, and I can't think of an example.
>
>>>> I understand the end goal is to keep consistency for the entire ASPM
>>>> logic. However I can't think of any possible solution right now.
>>>>
>>>>> - If we built with CONFIG_PCIEASPM_POWERSAVE=y, would that solve the
>>>>> SoC power state problem?
>>>>
>>>> Yes.
>>>>
>>>>> - What issues would CONFIG_PCIEASPM_POWERSAVE=y introduce?
>>>>
>>>> This will break many systems, at least for the 1st Gen Ryzen
>>>> desktops and laptops.
>>>>
>>>> All PCIe ASPM are not enabled by BIOS, and those systems immediately
>>>> freeze once ASPM is enabled.
>>>
>>> That indicates a defect in the Linux ASPM code. We should fix that.
>>> It should be safe to use CONFIG_PCIEASPM_POWERSAVE=y on every system.
>>
>> On those systems ASPM are also not enabled on Windows. So I think
>> ASPM are disabled for a reason.
>
> If the platform knows ASPM needs to be disabled, it should be using
> ACPI_FADT_NO_ASPM or _OSC to prevent the OS from using it. And if
> CONFIG_PCIEASPM_POWERSAVE=y means Linux enables ASPM when it
> shouldn't, that's a Linux bug that we need to fix.
Yes that's a bug which fixed by Ian's new patch.
>
>>> Are there bug reports for these? The info we would need to start with
>>> includes "lspci -vv" and dmesg log (with CONFIG_PCIEASPM_DEFAULT=y).
>>> If a console log with CONFIG_PCIEASPM_POWERSAVE=y is available, that
>>> might be interesting, too. We'll likely need to add some
>>> instrumentation and do some experimentation, but in principle, this
>>> should be fixable.
>>
>> Doing this is asking users to use hardware settings that ODM/OEM
>> never tested, and I think the risk is really high.
>
> What? That's not what I said at all. I'm asking for information
> about these hangs so we can fix them. I'm not suggesting that you
> should switch to CONFIG_PCIEASPM_POWERSAVE=y for the distro.
Ah, I thought your suggestion is switching to CONFIG_PCIEASPM_POWERSAVE=y,
because I sense you want to use that to cover the VMD ASPM this patch tries to
solve.
Do we have a conclusion how to enable VMD ASPM with CONFIG_PCIEASPM_DEFAULT=y?
>
> Let's back up. You said:
>
> CONFIG_PCIEASPM_POWERSAVE=y ... will break many systems, at least
> for the 1st Gen Ryzen desktops and laptops.
>
> All PCIe ASPM are not enabled by BIOS, and those systems immediately
> freeze once ASPM is enabled.
>
> These system hangs might be caused by (1) some hardware issue that
> causes a hang when ASPM is enabled even if it is configured correctly
> or (2) Linux configuring ASPM incorrectly.
It's (2) here.
>
> For case (1), the platform should be using ACPI_FADT_NO_ASPM or _OSC
> to prevent the OS from enabling ASPM. Linux should pay attention to
> that even when CONFIG_PCIEASPM_POWERSAVE=y.
>
> If the platform *should* use these mechanisms but doesn't, the
> solution is a quirk, not the folklore that "we can't use
> CONFIG_PCIEASPM_POWERSAVE=y because it breaks some systems."
The platform in question doesn't prevent OS from enabling ASPM.
>
> For case (2), we should fix Linux so it configures ASPM correctly.
>
> We cannot use the build-time CONFIG_PCIEASPM settings to avoid these
> hangs. We need to fix the Linux run-time code so the system operates
> correctly no matter what CONFIG_PCIEASPM setting is used.
>
> We have sysfs knobs to control ASPM (see 72ea91afbfb0 ("PCI/ASPM: Add
> sysfs attributes for controlling ASPM link states")). They can do the
> same thing at run-time as CONFIG_PCIEASPM_POWERSAVE=y does at
> build-time. If those knobs cause hangs on 1st Gen Ryzen systems, we
> need to fix that.
Ian's patch solves the issue, at least for the systems I have.
Kai-Heng
>
> Bjorn
Re: [Regression] "tpm: Require that all digests are present in TCG_PCR_EVENT2 structures" causes null pointer dereference
> On Sep 30, 2020, at 10:20, Jarkko Sakkinen > wrote: > > On Tue, Sep 29, 2020 at 01:52:04PM -0400, Mimi Zohar wrote: >> On Mon, 2020-09-28 at 22:16 +0800, Kai-Heng Feng wrote: >>> Hi Jarkko, >>> >>>> On Sep 28, 2020, at 22:06, Jarkko Sakkinen >>>> wrote: >>>> >>>> On Mon, Sep 28, 2020 at 08:31:04PM +0800, Kai-Heng Feng wrote: >>>>> Commit 7f3d176f5f7e "tpm: Require that all digests are present in >>>>> TCG_PCR_EVENT2 structures" causes a null pointer dereference on all >>>>> laptops I have: >>>> >>>> ... >>>> >>>>> [ 17.868849] BUG: kernel NULL pointer dereference, address: >>>>> 002c >>>>> [ 17.868852] #PF: supervisor read access in kernel mode >>>>> [ 17.868854] #PF: error_code(0x) - not-present page >>>>> [ 17.868855] PGD 0 P4D 0 >>>>> [ 17.868858] Oops: [#1] SMP PTI >>>>> [ 17.868860] CPU: 0 PID: 1873 Comm: fwupd Not tainted 5.8.0-rc6+ #25 >>>>> [ 17.868861] Hardware name: LENOVO 20LAZ3TXCN/20LAZ3TXCN, BIOS N27ET38W >>>>> (1.24 ) 11/28/2019 >>>>> [ 17.868866] RIP: 0010:tpm2_bios_measurements_start+0x38/0x1f0 >>>>> [ 17.868868] Code: 55 41 54 53 48 83 ec 30 4c 8b 16 65 48 8b 04 25 28 >>>>> 00 00 00 48 89 45 d0 48 8b 47 70 4c 8b a0 d0 06 00 00 48 8b 88 d8 06 00 >>>>> 00 <41> 8b 5c 24 1c 48 89 4d b0 48 89 d8 48 83 c3 20 4d 85 d2 75 31 4c >>>>> [ 17.868869] RSP: 0018:9da500a9fde0 EFLAGS: 00010282 >>>>> [ 17.868871] RAX: 917d03dc4000 RBX: RCX: >>>>> 0010 >>>>> [ 17.868872] RDX: 1000 RSI: 917c99b19460 RDI: >>>>> 917c99b19438 >>>>> [ 17.868873] RBP: 9da500a9fe38 R08: bda4ffa33fc0 R09: >>>>> 917cbfeae4c0 >>>>> [ 17.868874] R10: R11: 0002 R12: >>>>> 0010 >>>>> [ 17.868875] R13: 917c99b19438 R14: 917c99b19460 R15: >>>>> 917c99b19470 >>>>> [ 17.868876] FS: 7f9d80988b00() GS:917d0740() >>>>> knlGS: >>>>> [ 17.868877] CS: 0010 DS: ES: CR0: 80050033 >>>>> [ 17.868878] CR2: 002c CR3: 000219b12004 CR4: >>>>> 003606f0 >>>>> [ 17.868879] Call Trace: >>>>> [ 17.868884] seq_read+0x95/0x470 >>>>> [ 17.868887] ? security_file_permission+0x150/0x160 >>>>> [ 17.868889] vfs_read+0xaa/0x190 >>>>> [ 17.868891] ksys_read+0x67/0xe0 >>>>> [ 17.868893] __x64_sys_read+0x1a/0x20 >>>>> [ 17.868896] do_syscall_64+0x52/0xc0 >>>>> [ 17.868898] entry_SYSCALL_64_after_hwframe+0x44/0xa9 >>>>> [ 17.868900] RIP: 0033:0x7f9d83be91dc >>>>> [ 17.868901] Code: Bad RIP value. >>>>> [ 17.868902] RSP: 002b:7fff7f5e0250 EFLAGS: 0246 ORIG_RAX: >>>>> >>>>> [ 17.868903] RAX: ffda RBX: 5651d262f420 RCX: >>>>> 7f9d83be91dc >>>>> [ 17.868904] RDX: 1000 RSI: 7fff7f5e0350 RDI: >>>>> 0010 >>>>> [ 17.868905] RBP: 7f9d83cc54a0 R08: R09: >>>>> 5651d26c1830 >>>>> [ 17.868906] R10: 5651d2582010 R11: 0246 R12: >>>>> 1000 >>>>> [ 17.868907] R13: 7fff7f5e0350 R14: 0d68 R15: >>>>> 7f9d83cc48a0 >>>>> [ 17.868909] Modules linked in: rfcomm ccm cmac algif_hash >>>>> algif_skcipher af_alg snd_hda_codec_hdmi snd_hda_codec_realtek >>>>> snd_hda_codec_generic bnep joydev mei_hdcp wmi_bmof intel_rapl_msr >>>>> intel_wmi_thunderbolt x86_pkg_temp_thermal intel_powerclamp coretemp >>>>> nls_iso8859_1 kvm_intel kvm crct10dif_pclmul crc32_pclmul >>>>> ghash_clmulni_intel aesni_intel glue_helper crypto_simd cryptd rapl >>>>> input_leds intel_cstate snd_hda_intel snd_intel_dspcfg rmi_smbus iwlmvm >>>>> snd_hda_codec serio_raw snd_hwdep mac80211 rmi_core snd_hda_core libarc4 >>>>> uvcvideo snd_pcm videobuf2_vmalloc btusb videobuf2_memops iwlwifi >>>>> videobuf2_v4l2 btrtl btbcm videobuf2_common btintel thunderbolt i915
Re: [PATCH] drm/i915/lspcon: Limits to 8 bpc for RGB/YCbCr444
Hi Ville,
> On Aug 27, 2020, at 12:24 AM, Ville Syrjälä
> wrote:
>
> On Wed, Aug 26, 2020 at 01:21:15PM +0800, Kai-Heng Feng wrote:
>> LSPCON only supports 8 bpc for RGB/YCbCr444.
>>
>> Set the correct bpp otherwise it renders blank screen.
>
> Hmm. Does
> git://github.com/vsyrjala/linux.git dp_downstream_ports_5
> work?
>
> Actually better make that dp_downstream_ports_5^^^ aka.
> 54d846ce62a2 ("drm/i915: Do YCbCr 444->420 conversion via DP protocol
> converters") to avoid the experiments and hacks I have sitting on top.
Can you please rebase it to mainline master or drm-tip?
I am getting errors on the branch:
DESCEND objtool
CALLscripts/atomic/check-atomics.sh
CALLscripts/checksyscalls.sh
CHK include/generated/compile.h
Building modules, stage 2.
MODPOST 166 modules
LD arch/x86/boot/compressed/vmlinux
ld: arch/x86/boot/compressed/pgtable_64.o:(.bss+0x0): multiple definition of
`__force_order'; arch/x86/boot/compressed/kaslr_64.o:(.bss+0x0): first defined
here
ld: arch/x86/boot/compressed/head_64.o: warning: relocation in read-only
section `.head.text'
ld: warning: creating DT_TEXTREL in a PIE
make[2]: *** [arch/x86/boot/compressed/Makefile:119:
arch/x86/boot/compressed/vmlinux] Error 1
make[1]: *** [arch/x86/boot/Makefile:113: arch/x86/boot/compressed/vmlinux]
Error 2
make: *** [arch/x86/Makefile:284: bzImage] Error 2
make: *** Waiting for unfinished jobs
Kai-Heng
>
>>
>> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2195
>> Signed-off-by: Kai-Heng Feng
>> ---
>> drivers/gpu/drm/i915/display/intel_lspcon.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_lspcon.c
>> b/drivers/gpu/drm/i915/display/intel_lspcon.c
>> index b781bf469644..c7a44fcaade8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_lspcon.c
>> +++ b/drivers/gpu/drm/i915/display/intel_lspcon.c
>> @@ -196,7 +196,8 @@ void lspcon_ycbcr420_config(struct drm_connector
>> *connector,
>> crtc_state->port_clock /= 2;
>> crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR444;
>> crtc_state->lspcon_downsampling = true;
>> -}
>> +} else
>> +crtc_state->pipe_bpp = 24;
>> }
>>
>> static bool lspcon_probe(struct intel_lspcon *lspcon)
>> --
>> 2.17.1
>
> --
> Ville Syrjälä
> Intel
Re: [PATCH] ALSA: hda/hdmi: Use force connectivity quirk on another HP desktop
Hi,
> On Aug 11, 2020, at 17:53, Kai-Heng Feng wrote:
>
> There's another HP desktop has buggy BIOS which flags the Port
> Connectivity bit as no connection.
>
> Apply force connectivity quirk to enable DP/HDMI audio.
>
> Signed-off-by: Kai-Heng Feng
I guess this patch was omitted as well...
Kai-Heng
> ---
> sound/pci/hda/patch_hdmi.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 4bbd12d3b1b5..b8c8490e568b 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1863,6 +1863,7 @@ static int hdmi_add_cvt(struct hda_codec *codec,
> hda_nid_t cvt_nid)
> }
>
> static const struct snd_pci_quirk force_connect_list[] = {
> + SND_PCI_QUIRK(0x103c, 0x870f, "HP", 1),
> SND_PCI_QUIRK(0x103c, 0x871a, "HP", 1),
> {}
> };
> --
> 2.17.1
>
Re: [PATCH] ALSA: hda/hdmi: Use force connectivity quirk on another HP desktop
> On Aug 12, 2020, at 23:47, Takashi Iwai wrote:
>
> On Wed, 12 Aug 2020 17:43:27 +0200,
> Kai-Heng Feng wrote:
>>
>> Hi,
>>
>>> On Aug 11, 2020, at 17:53, Kai-Heng Feng
>>> wrote:
>>>
>>> There's another HP desktop has buggy BIOS which flags the Port
>>> Connectivity bit as no connection.
>>>
>>> Apply force connectivity quirk to enable DP/HDMI audio.
>>>
>>> Signed-off-by: Kai-Heng Feng
>>
>> I guess this patch was omitted as well...
>
> Not omitted but applied to a wrong internal branch, sorry.
> Now I rebased to the proper branch and pushed out.
Thanks a lot!
Kai-Heng
>
>
> thanks,
>
> Takashi
>
>>
>> Kai-Heng
>>
>>> ---
>>> sound/pci/hda/patch_hdmi.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
>>> index 4bbd12d3b1b5..b8c8490e568b 100644
>>> --- a/sound/pci/hda/patch_hdmi.c
>>> +++ b/sound/pci/hda/patch_hdmi.c
>>> @@ -1863,6 +1863,7 @@ static int hdmi_add_cvt(struct hda_codec *codec,
>>> hda_nid_t cvt_nid)
>>> }
>>>
>>> static const struct snd_pci_quirk force_connect_list[] = {
>>> + SND_PCI_QUIRK(0x103c, 0x870f, "HP", 1),
>>> SND_PCI_QUIRK(0x103c, 0x871a, "HP", 1),
>>> {}
>>> };
>>> --
>>> 2.17.1
>>>
>>
Re: [PATCH] rtw88: pci: Power cycle device during shutdown
> On Aug 26, 2020, at 08:27, Brian Norris wrote: > > On Mon, Aug 24, 2020 at 2:32 AM Kai-Heng Feng > wrote: >> >> Sometimes system freeze on cold/warm boot when rtw88 is probing. >> >> According to [1], platform firmware may not properly power manage the >> device during shutdown. I did some expirements and putting the device to >> D3 can workaround the issue. >> >> So let's power cycle the device by putting the device to D3 at shutdown >> to prevent the issue from happening. >> >> [1] https://bugzilla.kernel.org/show_bug.cgi?id=206411#c9 >> >> Signed-off-by: Kai-Heng Feng > > Can you at least include some more details, like which hardware > version and firmware? 8723DE, 8822BE and 8822CE are affected [1]. > And which platform? Many x86 laptops. Some users claim BIOS update can fix the issue, however some are still affected. > It seems a bit harsh to > include a platform workaround to run for everyone, unless there's > truly no downside. But even then, it's good to log what you're working > with, in case there are ever problems with this in the future. Ok. I can send V2 with more detailed commit message. [1] https://bugs.launchpad.net/bugs/1872984 Kai-Heng > > Brian
[PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
Suspend with s2idle or by the following steps cause screen frozen:
# echo devices > /sys/power/pm_test
# echo freeze > /sys/power/mem
[ 289.625461] [drm:uvd_v1_0_ib_test [radeon]] *ERROR* radeon: fence wait timed
out.
[ 289.625494] [drm:radeon_ib_ring_tests [radeon]] *ERROR* radeon: failed
testing IB on ring 5 (-110).
The issue doesn't happen on traditional S3, probably because firmware or
hardware provides extra power management.
Inspired by Daniel Drake's patch [1] on amdgpu, using a similar approach
can fix the issue.
[1] https://patchwork.freedesktop.org/patch/335839/
Signed-off-by: Kai-Heng Feng
---
drivers/gpu/drm/radeon/radeon_device.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_device.c
b/drivers/gpu/drm/radeon/radeon_device.c
index 266e3cbbd09b..df823b9ad79f 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
#include
@@ -1643,6 +1644,8 @@ int radeon_suspend_kms(struct drm_device *dev, bool
suspend,
rdev->asic->asic_reset(rdev, true);
pci_restore_state(dev->pdev);
} else if (suspend) {
+ if (pm_suspend_no_platform())
+ rdev->asic->asic_reset(rdev, true);
/* Shut down the device */
pci_disable_device(dev->pdev);
pci_set_power_state(dev->pdev, PCI_D3hot);
--
2.17.1
Re: [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
> On Sep 1, 2020, at 22:19, Alex Deucher wrote:
>
> On Tue, Sep 1, 2020 at 3:32 AM Kai-Heng Feng
> wrote:
>>
>> Suspend with s2idle or by the following steps cause screen frozen:
>> # echo devices > /sys/power/pm_test
>> # echo freeze > /sys/power/mem
>>
>> [ 289.625461] [drm:uvd_v1_0_ib_test [radeon]] *ERROR* radeon: fence wait
>> timed out.
>> [ 289.625494] [drm:radeon_ib_ring_tests [radeon]] *ERROR* radeon: failed
>> testing IB on ring 5 (-110).
>>
>> The issue doesn't happen on traditional S3, probably because firmware or
>> hardware provides extra power management.
>>
>> Inspired by Daniel Drake's patch [1] on amdgpu, using a similar approach
>> can fix the issue.
>
> It doesn't actually fix the issue. The device is never powered down
> so you are using more power than you would if you did not suspend in
> the first place. The reset just works around the fact that the device
> is never powered down.
So how do we properly suspend/resume the device without help from platform
firmware?
Kai-Heng
>
> Alex
>
>>
>> [1] https://patchwork.freedesktop.org/patch/335839/
>>
>> Signed-off-by: Kai-Heng Feng
>> ---
>> drivers/gpu/drm/radeon/radeon_device.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_device.c
>> b/drivers/gpu/drm/radeon/radeon_device.c
>> index 266e3cbbd09b..df823b9ad79f 100644
>> --- a/drivers/gpu/drm/radeon/radeon_device.c
>> +++ b/drivers/gpu/drm/radeon/radeon_device.c
>> @@ -33,6 +33,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -1643,6 +1644,8 @@ int radeon_suspend_kms(struct drm_device *dev, bool
>> suspend,
>>rdev->asic->asic_reset(rdev, true);
>>pci_restore_state(dev->pdev);
>>} else if (suspend) {
>> + if (pm_suspend_no_platform())
>> + rdev->asic->asic_reset(rdev, true);
>>/* Shut down the device */
>>pci_disable_device(dev->pdev);
>>pci_set_power_state(dev->pdev, PCI_D3hot);
>> --
>> 2.17.1
>>
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/radeon: Reset ASIC if suspend is not managed by platform firmware
> On Sep 2, 2020, at 00:30, Alex Deucher wrote:
>
> On Tue, Sep 1, 2020 at 12:21 PM Kai-Heng Feng
> wrote:
>>
>>
>>
>>> On Sep 1, 2020, at 22:19, Alex Deucher wrote:
>>>
>>> On Tue, Sep 1, 2020 at 3:32 AM Kai-Heng Feng
>>> wrote:
>>>>
>>>> Suspend with s2idle or by the following steps cause screen frozen:
>>>> # echo devices > /sys/power/pm_test
>>>> # echo freeze > /sys/power/mem
>>>>
>>>> [ 289.625461] [drm:uvd_v1_0_ib_test [radeon]] *ERROR* radeon: fence wait
>>>> timed out.
>>>> [ 289.625494] [drm:radeon_ib_ring_tests [radeon]] *ERROR* radeon: failed
>>>> testing IB on ring 5 (-110).
>>>>
>>>> The issue doesn't happen on traditional S3, probably because firmware or
>>>> hardware provides extra power management.
>>>>
>>>> Inspired by Daniel Drake's patch [1] on amdgpu, using a similar approach
>>>> can fix the issue.
>>>
>>> It doesn't actually fix the issue. The device is never powered down
>>> so you are using more power than you would if you did not suspend in
>>> the first place. The reset just works around the fact that the device
>>> is never powered down.
>>
>> So how do we properly suspend/resume the device without help from platform
>> firmware?
>
> I guess you don't?
Unfortunate but I guess we need to accept reality and use the default suspend
method.
Kai-Heng
>
> Alex
>
>
>>
>> Kai-Heng
>>
>>>
>>> Alex
>>>
>>>>
>>>> [1] https://patchwork.freedesktop.org/patch/335839/
>>>>
>>>> Signed-off-by: Kai-Heng Feng
>>>> ---
>>>> drivers/gpu/drm/radeon/radeon_device.c | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_device.c
>>>> b/drivers/gpu/drm/radeon/radeon_device.c
>>>> index 266e3cbbd09b..df823b9ad79f 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_device.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_device.c
>>>> @@ -33,6 +33,7 @@
>>>> #include
>>>> #include
>>>> #include
>>>> +#include
>>>>
>>>> #include
>>>> #include
>>>> @@ -1643,6 +1644,8 @@ int radeon_suspend_kms(struct drm_device *dev, bool
>>>> suspend,
>>>> rdev->asic->asic_reset(rdev, true);
>>>> pci_restore_state(dev->pdev);
>>>> } else if (suspend) {
>>>> + if (pm_suspend_no_platform())
>>>> + rdev->asic->asic_reset(rdev, true);
>>>> /* Shut down the device */
>>>> pci_disable_device(dev->pdev);
>>>> pci_set_power_state(dev->pdev, PCI_D3hot);
>>>> --
>>>> 2.17.1
>>>>
>>>> ___
>>>> dri-devel mailing list
>>>> dri-de...@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
[PATCH] ALSA: usb-audio: Add support for Lenovo ThinkStation P620
Lenovo ThinkStation P620 is like other TRX40 boards, is equipped with
two USB audio cards.
USB device (17aa:104d) provides functionality for Internal Speaker and
Front Headset. It's UAC v2, so it supports insertion control (jack
detection). However, when trying to get the connector status of the
speaker, an error occurs:
[5.787405] usb 3-1: cannot get connectors status: req = 0x81, wValue =
0x200, wIndex = 0x1000, type = 0
Since the insertion control works perfectly for the headset, the error
for speaker is probably casued by connecting internally. So let's relax
the error for a bit if it's a speaker, and always reports it's connected.
USB device (17aa:1046) is for rear Line-in, Line-out and Microphone.
The insertion control works for all three jacks. However, there's an
Function Unit that doesn't work:
[5.905415] usb 3-6: cannot get ctl value: req = 0x83, wValue = 0xc00,
wIndex = 0x1300, type = 4
[5.905418] usb 3-6: 19:0: cannot get min/max values for control 12 (id 19)
So turn off the FU to avoid the error.
Also, add specific card name for both devices, so userspace can easily
indentify both cards.
Signed-off-by: Kai-Heng Feng
---
sound/usb/mixer.c| 4
sound/usb/mixer_maps.c | 9 +
sound/usb/quirks-table.h | 13 +
3 files changed, 26 insertions(+)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index eab0fd4fd7c3..6b0f3a8469ef 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -1463,6 +1463,10 @@ static int mixer_ctl_connector_get(struct snd_kcontrol
*kcontrol,
snd_usb_unlock_shutdown(chip);
if (ret < 0) {
+ if (strstr(kcontrol->id.name, "Speaker")) {
+ ucontrol->value.integer.value[0] = 1;
+ return 0;
+ }
error:
usb_audio_err(chip,
"cannot get connectors status: req = %#x, wValue = %#x,
wIndex = %#x, type = %d\n",
diff --git a/sound/usb/mixer_maps.c b/sound/usb/mixer_maps.c
index 9af7aa93f6fa..08eb230e2bff 100644
--- a/sound/usb/mixer_maps.c
+++ b/sound/usb/mixer_maps.c
@@ -370,6 +370,11 @@ static const struct usbmix_name_map asus_rog_map[] = {
{}
};
+static const struct usbmix_name_map lenovo_p620_rear_map[] = {
+ { 19, NULL, 12 }, /* FU, Input Gain Pad */
+ {}
+};
+
/* TRX40 mobos with Realtek ALC1220-VB */
static const struct usbmix_name_map trx40_mobo_map[] = {
{ 18, NULL }, /* OT, IEC958 - broken response, disabled */
@@ -573,6 +578,10 @@ static const struct usbmix_ctl_map usbmix_ctl_maps[] = {
.map = trx40_mobo_map,
.connector_map = trx40_mobo_connector_map,
},
+ { /* Lenovo ThinkStation P620 Rear */
+ .id = USB_ID(0x17aa, 0x1046),
+ .map = lenovo_p620_rear_map,
+ },
{ 0 } /* terminator */
};
diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h
index 9092cc0aa807..09590f055a1d 100644
--- a/sound/usb/quirks-table.h
+++ b/sound/usb/quirks-table.h
@@ -2794,6 +2794,19 @@ YAMAHA_DEVICE(0x7010, "UB99"),
QUIRK_RENAME_DEVICE("Rane", "SL-1")
},
+/* Lenovo ThinkStation P620 Rear Line-in, Line-out and Microphone */
+{
+ USB_DEVICE(0x17aa, 0x1046),
+ QUIRK_DEVICE_PROFILE("Lenovo", "ThinkStation P620 Rear",
+"Lenovo-ThinkStation-P620-Rear"),
+},
+/* Lenovo ThinkStation P620 Internal Speaker + Front Headset */
+{
+ USB_DEVICE(0x17aa, 0x104d),
+ QUIRK_DEVICE_PROFILE("Lenovo", "ThinkStation P620 Main",
+"Lenovo-ThinkStation-P620-Main"),
+},
+
/* Native Instruments MK2 series */
{
/* Komplete Audio 6 */
--
2.17.1
[PATCH] ALSA: usb-audio: Disable autosuspend for Lenovo ThinkStation P620
If USB autosuspend is enabled, both front and rear panel can no longer
detect jack insertion.
Enable USB remote wakeup, i.e. needs_remote_wakeup = 1, doesn't help
either.
So disable USB autosuspend to prevent missing jack detection event.
Signed-off-by: Kai-Heng Feng
---
sound/usb/quirks-table.h | 18 ++
sound/usb/quirks.c | 10 ++
sound/usb/usbaudio.h | 1 +
3 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h
index f4fb002e3ef4..416de71c6895 100644
--- a/sound/usb/quirks-table.h
+++ b/sound/usb/quirks-table.h
@@ -2827,14 +2827,24 @@ YAMAHA_DEVICE(0x7010, "UB99"),
/* Lenovo ThinkStation P620 Rear Line-in, Line-out and Microphone */
{
USB_DEVICE(0x17aa, 0x1046),
- QUIRK_DEVICE_PROFILE("Lenovo", "ThinkStation P620 Rear",
-"Lenovo-ThinkStation-P620-Rear"),
+ .driver_info = (unsigned long) & (const struct snd_usb_audio_quirk) {
+ .vendor_name = "Lenovo",
+ .product_name = "ThinkStation P620 Rear",
+ .profile_name = "Lenovo-ThinkStation-P620-Rear",
+ .ifnum = QUIRK_ANY_INTERFACE,
+ .type = QUIRK_SETUP_DISABLE_AUTOSUSPEND
+ }
},
/* Lenovo ThinkStation P620 Internal Speaker + Front Headset */
{
USB_DEVICE(0x17aa, 0x104d),
- QUIRK_DEVICE_PROFILE("Lenovo", "ThinkStation P620 Main",
-"Lenovo-ThinkStation-P620-Main"),
+ .driver_info = (unsigned long) & (const struct snd_usb_audio_quirk) {
+ .vendor_name = "Lenovo",
+ .product_name = "ThinkStation P620 Main",
+ .profile_name = "Lenovo-ThinkStation-P620-Main",
+ .ifnum = QUIRK_ANY_INTERFACE,
+ .type = QUIRK_SETUP_DISABLE_AUTOSUSPEND
+ }
},
/* Native Instruments MK2 series */
diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index abf99b814a0f..b800fd92106c 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -518,6 +518,15 @@ static int setup_fmt_after_resume_quirk(struct
snd_usb_audio *chip,
return 1; /* Continue with creating streams and mixer */
}
+static int setup_disable_autosuspend(struct snd_usb_audio *chip,
+ struct usb_interface *iface,
+ struct usb_driver *driver,
+ const struct snd_usb_audio_quirk *quirk)
+{
+ driver->supports_autosuspend = 0;
+ return 1; /* Continue with creating streams and mixer */
+}
+
/*
* audio-interface quirks
*
@@ -557,6 +566,7 @@ int snd_usb_create_quirk(struct snd_usb_audio *chip,
[QUIRK_AUDIO_ALIGN_TRANSFER] = create_align_transfer_quirk,
[QUIRK_AUDIO_STANDARD_MIXER] = create_standard_mixer_quirk,
[QUIRK_SETUP_FMT_AFTER_RESUME] = setup_fmt_after_resume_quirk,
+ [QUIRK_SETUP_DISABLE_AUTOSUSPEND] = setup_disable_autosuspend,
};
if (quirk->type < QUIRK_TYPE_COUNT) {
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index b91c4c0807ec..6839915a0128 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -102,6 +102,7 @@ enum quirk_type {
QUIRK_AUDIO_ALIGN_TRANSFER,
QUIRK_AUDIO_STANDARD_MIXER,
QUIRK_SETUP_FMT_AFTER_RESUME,
+ QUIRK_SETUP_DISABLE_AUTOSUSPEND,
QUIRK_TYPE_COUNT
};
--
2.17.1
[PATCH] rtw88: pci: Power cycle device during shutdown
Sometimes system freeze on cold/warm boot when rtw88 is probing. According to [1], platform firmware may not properly power manage the device during shutdown. I did some expirements and putting the device to D3 can workaround the issue. So let's power cycle the device by putting the device to D3 at shutdown to prevent the issue from happening. [1] https://bugzilla.kernel.org/show_bug.cgi?id=206411#c9 Signed-off-by: Kai-Heng Feng --- drivers/net/wireless/realtek/rtw88/pci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c index 3413973bc475..7f1f5073b9f4 100644 --- a/drivers/net/wireless/realtek/rtw88/pci.c +++ b/drivers/net/wireless/realtek/rtw88/pci.c @@ -1599,6 +1599,8 @@ void rtw_pci_shutdown(struct pci_dev *pdev) if (chip->ops->shutdown) chip->ops->shutdown(rtwdev); + + pci_set_power_state(pdev, PCI_D3hot); } EXPORT_SYMBOL(rtw_pci_shutdown); -- 2.17.1
[PATCH] ALSA: hda/realtek: Enable front panel headset LED on Lenovo ThinkStation P520
On Lenovo P520, the front panel headset LED isn't lit up right now.
Realtek states that the LED needs to be enabled by ALC233's GPIO2, so
let's do it accordingly to light the LED up.
Signed-off-by: Kai-Heng Feng
---
sound/pci/hda/patch_realtek.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index c521a1f17096..ba941bd0b792 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -6017,6 +6017,7 @@ static void alc_fixup_thinkpad_acpi(struct hda_codec
*codec,
#include "hp_x360_helper.c"
enum {
+ ALC269_FIXUP_GPIO2,
ALC269_FIXUP_SONY_VAIO,
ALC275_FIXUP_SONY_VAIO_GPIO2,
ALC269_FIXUP_DELL_M101Z,
@@ -6194,6 +6195,10 @@ enum {
};
static const struct hda_fixup alc269_fixups[] = {
+ [ALC269_FIXUP_GPIO2] = {
+ .type = HDA_FIXUP_FUNC,
+ .v.func = alc_fixup_gpio2,
+ },
[ALC269_FIXUP_SONY_VAIO] = {
.type = HDA_FIXUP_PINCTLS,
.v.pins = (const struct hda_pintbl[]) {
@@ -7013,6 +7018,8 @@ static const struct hda_fixup alc269_fixups[] = {
[ALC233_FIXUP_LENOVO_MULTI_CODECS] = {
.type = HDA_FIXUP_FUNC,
.v.func = alc233_alc662_fixup_lenovo_dual_codecs,
+ .chained = true,
+ .chain_id = ALC269_FIXUP_GPIO2
},
[ALC233_FIXUP_ACER_HEADSET_MIC] = {
.type = HDA_FIXUP_VERBS,
--
2.17.1
Re: [PATCH] ALSA: hda/realtek: Enable front panel headset LED on Lenovo ThinkStation P520
Hi Hui,
> On Sep 14, 2020, at 16:04, Hui Wang wrote:
>
> Thanks Kaiheng, and we just had one P520 in the Beijing office and I also
> worked on this issue happenly. Does the led change according to jack plugging
> in or plugging out with your patch?
No, the LED won't reflect the jack plugging status.
The LED is always on under Windows, so we are doing the same here.
Kai-Heng
> I also prepared a patchset but my patchset has more code than yours, please
> take a look. :-)
>
> Thanks.
>
> Hui.
>
> On 2020/9/14 下午3:02, Kai-Heng Feng wrote:
>> On Lenovo P520, the front panel headset LED isn't lit up right now.
>>
>> Realtek states that the LED needs to be enabled by ALC233's GPIO2, so
>> let's do it accordingly to light the LED up.
>>
>> Signed-off-by: Kai-Heng Feng
>> ---
>> sound/pci/hda/patch_realtek.c | 7 +++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
>> index c521a1f17096..ba941bd0b792 100644
>> --- a/sound/pci/hda/patch_realtek.c
>> +++ b/sound/pci/hda/patch_realtek.c
>> @@ -6017,6 +6017,7 @@ static void alc_fixup_thinkpad_acpi(struct hda_codec
>> *codec,
>> #include "hp_x360_helper.c"
>>enum {
>> +ALC269_FIXUP_GPIO2,
>> ALC269_FIXUP_SONY_VAIO,
>> ALC275_FIXUP_SONY_VAIO_GPIO2,
>> ALC269_FIXUP_DELL_M101Z,
>> @@ -6194,6 +6195,10 @@ enum {
>> };
>>static const struct hda_fixup alc269_fixups[] = {
>> +[ALC269_FIXUP_GPIO2] = {
>> +.type = HDA_FIXUP_FUNC,
>> +.v.func = alc_fixup_gpio2,
>> +},
>> [ALC269_FIXUP_SONY_VAIO] = {
>> .type = HDA_FIXUP_PINCTLS,
>> .v.pins = (const struct hda_pintbl[]) {
>> @@ -7013,6 +7018,8 @@ static const struct hda_fixup alc269_fixups[] = {
>> [ALC233_FIXUP_LENOVO_MULTI_CODECS] = {
>> .type = HDA_FIXUP_FUNC,
>> .v.func = alc233_alc662_fixup_lenovo_dual_codecs,
>> +.chained = true,
>> +.chain_id = ALC269_FIXUP_GPIO2
>> },
>> [ALC233_FIXUP_ACER_HEADSET_MIC] = {
>> .type = HDA_FIXUP_VERBS,
[PATCH] Revert "ALSA: usb-audio: Disable Lenovo P620 Rear line-in volume control"
This reverts commit 34dedd2a83b241ba6aeb290260313c65dc58660e.
According to Realtek, volume FU works for line-in.
I can confirm volume control works after device firmware is updated.
Signed-off-by: Kai-Heng Feng
---
sound/usb/mixer_maps.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/sound/usb/mixer_maps.c b/sound/usb/mixer_maps.c
index 5b43e9e40e49..c369c81e74c4 100644
--- a/sound/usb/mixer_maps.c
+++ b/sound/usb/mixer_maps.c
@@ -371,7 +371,6 @@ static const struct usbmix_name_map asus_rog_map[] = {
};
static const struct usbmix_name_map lenovo_p620_rear_map[] = {
- { 19, NULL, 2 }, /* FU, Volume */
{ 19, NULL, 12 }, /* FU, Input Gain Pad */
{}
};
--
2.17.1
[PATCH] ALSA: hda: Resume codec for system suspend if LED is controlled by codec
Laptop with codec controlled LEDs takes a very long time to suspend
after commit 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use
direct-complete optimization"):
[ 90.065964] PM: suspend entry (s2idle)
[ 90.067337] Filesystems sync: 0.001 seconds
[ 90.185758] Freezing user space processes ... (elapsed 0.002 seconds) done.
[ 90.188713] OOM killer disabled.
[ 90.188714] Freezing remaining freezable tasks ... (elapsed 0.001 seconds)
done.
[ 90.190024] printk: Suspending console(s) (use no_console_suspend to debug)
[ 90.904912] intel_pch_thermal :00:12.0: CPU-PCH is cool [49C], continue
to suspend
[ 321.262505] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register
0x2b8000. -5
[ 328.426919] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register
0x2b8000. -5
[ 329.490933] ACPI: EC: interrupt blocked
That commit keeps codec suspended during the system suspend. This
doesn't play well with codec controlled mute and micmute LEDs, because
LED core will use codec registers to turn off those LEDs.
Currently, all users of create_mute_led_cdev() use codec to control
LED, so unconditionally runtime resume those codecs before system
suspend to solve the problem.
Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete
optimization")
Signed-off-by: Kai-Heng Feng
---
include/sound/hda_codec.h | 1 +
sound/pci/hda/hda_codec.c | 7 +++
sound/pci/hda/hda_generic.c | 1 +
3 files changed, 9 insertions(+)
diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index 2e8d51937acd..b01d76abe008 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -255,6 +255,7 @@ struct hda_codec {
unsigned int relaxed_resume:1; /* don't resume forcibly for jack */
unsigned int forced_resume:1; /* forced resume for jack */
unsigned int mst_no_extra_pcms:1; /* no backup PCMs for DP-MST */
+ unsigned int resume_for_sleep:1; /* runtime resume for system sleep */
#ifdef CONFIG_PM
unsigned long power_on_acct;
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 687216e74526..b890d9b4339e 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -2983,6 +2983,13 @@ static int hda_codec_runtime_resume(struct device *dev)
#ifdef CONFIG_PM_SLEEP
static int hda_codec_pm_prepare(struct device *dev)
{
+ struct hda_codec *codec = dev_to_hda_codec(dev);
+
+ if (codec->resume_for_sleep) {
+ pm_runtime_resume(dev);
+ return 0;
+ }
+
return pm_runtime_suspended(dev);
}
diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
index 8060cc86dfea..6d259d5bb5bb 100644
--- a/sound/pci/hda/hda_generic.c
+++ b/sound/pci/hda/hda_generic.c
@@ -3913,6 +3913,7 @@ static int create_mute_led_cdev(struct hda_codec *codec,
cdev->brightness_set_blocking = callback;
cdev->brightness = ledtrig_audio_get(micmute ? LED_AUDIO_MICMUTE :
LED_AUDIO_MUTE);
cdev->flags = LED_CORE_SUSPENDRESUME;
+ codec->resume_for_sleep = 1;
return devm_led_classdev_register(>core.dev, cdev);
}
--
2.29.2
Re: [PATCH] ALSA: hda: Resume codec for system suspend if LED is controlled by codec
On Sat, Dec 26, 2020 at 3:46 PM Takashi Iwai wrote:
>
> On Fri, 25 Dec 2020 17:47:23 +0100,
> Kai-Heng Feng wrote:
> >
> > Laptop with codec controlled LEDs takes a very long time to suspend
> > after commit 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use
> > direct-complete optimization"):
> > [ 90.065964] PM: suspend entry (s2idle)
> > [ 90.067337] Filesystems sync: 0.001 seconds
> > [ 90.185758] Freezing user space processes ... (elapsed 0.002 seconds)
> > done.
> > [ 90.188713] OOM killer disabled.
> > [ 90.188714] Freezing remaining freezable tasks ... (elapsed 0.001
> > seconds) done.
> > [ 90.190024] printk: Suspending console(s) (use no_console_suspend to
> > debug)
> > [ 90.904912] intel_pch_thermal :00:12.0: CPU-PCH is cool [49C],
> > continue to suspend
> > [ 321.262505] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register
> > 0x2b8000. -5
> > [ 328.426919] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register
> > 0x2b8000. -5
> > [ 329.490933] ACPI: EC: interrupt blocked
> >
> > That commit keeps codec suspended during the system suspend. This
> > doesn't play well with codec controlled mute and micmute LEDs, because
> > LED core will use codec registers to turn off those LEDs.
> >
> > Currently, all users of create_mute_led_cdev() use codec to control
> > LED, so unconditionally runtime resume those codecs before system
> > suspend to solve the problem.
> >
> > Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete
> > optimization")
> > Signed-off-by: Kai-Heng Feng
>
> A puzzling point is that it's applied only to the cases with the led
> cdev. Or does it basically apply for the ASoC controller?
> That is, if you run with legacy HDA (snd-intel-dspcfg.dsp_driver=1),
> does the issue appear as well on those machines?
No, the issue doesn't happen with snd-intel-dspcfg.dsp_driver=1. It
only happens when SOF driver is in use.
My analysis was wrong, I will send a new patch to address the root cause.
Kai-Heng
>
>
> thanks,
>
> Takashi
>
> > ---
> > include/sound/hda_codec.h | 1 +
> > sound/pci/hda/hda_codec.c | 7 +++
> > sound/pci/hda/hda_generic.c | 1 +
> > 3 files changed, 9 insertions(+)
> >
> > diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
> > index 2e8d51937acd..b01d76abe008 100644
> > --- a/include/sound/hda_codec.h
> > +++ b/include/sound/hda_codec.h
> > @@ -255,6 +255,7 @@ struct hda_codec {
> > unsigned int relaxed_resume:1; /* don't resume forcibly for jack */
> > unsigned int forced_resume:1; /* forced resume for jack */
> > unsigned int mst_no_extra_pcms:1; /* no backup PCMs for DP-MST */
> > + unsigned int resume_for_sleep:1; /* runtime resume for system sleep
> > */
> >
> > #ifdef CONFIG_PM
> > unsigned long power_on_acct;
> > diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> > index 687216e74526..b890d9b4339e 100644
> > --- a/sound/pci/hda/hda_codec.c
> > +++ b/sound/pci/hda/hda_codec.c
> > @@ -2983,6 +2983,13 @@ static int hda_codec_runtime_resume(struct device
> > *dev)
> > #ifdef CONFIG_PM_SLEEP
> > static int hda_codec_pm_prepare(struct device *dev)
> > {
> > + struct hda_codec *codec = dev_to_hda_codec(dev);
> > +
> > + if (codec->resume_for_sleep) {
> > + pm_runtime_resume(dev);
> > + return 0;
> > + }
> > +
> > return pm_runtime_suspended(dev);
> > }
> >
> > diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c
> > index 8060cc86dfea..6d259d5bb5bb 100644
> > --- a/sound/pci/hda/hda_generic.c
> > +++ b/sound/pci/hda/hda_generic.c
> > @@ -3913,6 +3913,7 @@ static int create_mute_led_cdev(struct hda_codec
> > *codec,
> > cdev->brightness_set_blocking = callback;
> > cdev->brightness = ledtrig_audio_get(micmute ? LED_AUDIO_MICMUTE :
> > LED_AUDIO_MUTE);
> > cdev->flags = LED_CORE_SUSPENDRESUME;
> > + codec->resume_for_sleep = 1;
> >
> > return devm_led_classdev_register(>core.dev, cdev);
> > }
> > --
> > 2.29.2
> >
Re: [PATCH 1/2] PCI/ASPM: Store disabled ASPM states
> On Dec 9, 2020, at 01:11, Heiner Kallweit wrote:
>
> Am 08.12.2020 um 09:25 schrieb Kai-Heng Feng:
>> If we use sysfs to disable L1 ASPM, then enable one L1 ASPM substate
>> again, all other substates will also be enabled too:
>>
>> link# grep . *
>> clkpm:1
>> l0s_aspm:1
>> l1_1_aspm:1
>> l1_1_pcipm:1
>> l1_2_aspm:1
>> l1_2_pcipm:1
>> l1_aspm:1
>>
>> link# echo 0 > l1_aspm
>>
>> link# grep . *
>> clkpm:1
>> l0s_aspm:1
>> l1_1_aspm:0
>> l1_1_pcipm:0
>> l1_2_aspm:0
>> l1_2_pcipm:0
>> l1_aspm:0
>>
>> link# echo 1 > l1_2_aspm
>>
>> link# grep . *
>> clkpm:1
>> l0s_aspm:1
>> l1_1_aspm:1
>> l1_1_pcipm:1
>> l1_2_aspm:1
>> l1_2_pcipm:1
>> l1_aspm:1
>>
>> This is because disabled ASPM states weren't saved, so enable any of the
>> substate will also enable others.
>>
>> So store the disabled ASPM states for consistency.
>>
>> Signed-off-by: Kai-Heng Feng
>> ---
>> drivers/pci/pcie/aspm.c | 10 --
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
>> index ac0557a305af..2ea9fddadfad 100644
>> --- a/drivers/pci/pcie/aspm.c
>> +++ b/drivers/pci/pcie/aspm.c
>> @@ -658,6 +658,8 @@ static void pcie_aspm_cap_init(struct pcie_link_state
>> *link, int blacklist)
>> /* Setup initial capable state. Will be updated later */
>> link->aspm_capable = link->aspm_support;
>>
>> +link->aspm_disable = link->aspm_capable & ~link->aspm_default;
>> +
>
> This makes sense only in combination with patch 2. However I think patch 1
> should be independent of patch 2. Especially if we consider patch 1 a fix
> that is applied to stable whilst patch 2 is an improvement for next.
>
>> /* Get and check endpoint acceptable latencies */
>> list_for_each_entry(child, >devices, bus_list) {
>> u32 reg32, encoding;
>> @@ -1226,11 +1228,15 @@ static ssize_t aspm_attr_store_common(struct device
>> *dev,
>> mutex_lock(_lock);
>>
>> if (state_enable) {
>> -link->aspm_disable &= ~state;
>> /* need to enable L1 for substates */
>> if (state & ASPM_STATE_L1SS)
>> -link->aspm_disable &= ~ASPM_STATE_L1;
>> +state |= ASPM_STATE_L1;
>> +
>> +link->aspm_disable &= ~state;
>
> I don't see what this part of the patch changes. Can you elaborate on why
> this is needed?
No this is just a cosmetic change. Of course "cosmetic" is really subjective.
I'll drop this part in v2.
>
>> } else {
>> +if (state == ASPM_STATE_L1)
>> +state |= ASPM_STATE_L1SS;
>> +
>
> I think this part should be sufficient to fix the behavior. because what
> I think currently happens:
>
> 1. original status: policy powersupersave, nothing disabled -> L1 + L1SS
> active
> 2. disable L1: L1 disabled, pcie_config_aspm_link() disabled L1 and L1SS
> w/o adding L1SS to link-> aspm_disabled
> 3. enable one L1SS state: aspm_attr_store_common() removes L1 from
> link->aspm_disabled -> link->aspm_disabled is empty, resulting in
> L1 + L1SS being active
Yes. This is the case the patch solves.
Kai-Heng
>
>> link->aspm_disable |= state;
>> }
Re: [PATCH 2/2] PCI/ASPM: Use capability to override ASPM via sysfs
> On Dec 9, 2020, at 01:18, Heiner Kallweit wrote: > > Am 08.12.2020 um 09:25 schrieb Kai-Heng Feng: >> If we are to use sysfs to change ASPM settings, we may want to override >> the default ASPM policy. >> >> So use ASPM capability, instead of default policy, to be able to use all >> possible ASPM states. >> >> Signed-off-by: Kai-Heng Feng >> --- >> drivers/pci/pcie/aspm.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c >> index 2ea9fddadfad..326da7bbc84d 100644 >> --- a/drivers/pci/pcie/aspm.c >> +++ b/drivers/pci/pcie/aspm.c >> @@ -1239,8 +1239,7 @@ static ssize_t aspm_attr_store_common(struct device >> *dev, >> >> link->aspm_disable |= state; >> } >> - >> -pcie_config_aspm_link(link, policy_to_aspm_state(link)); >> +pcie_config_aspm_link(link, link->aspm_capable); >> > I like the idea, because the policy can be changed by the user anyway. > Therefore I don't see it as a hard system limit. > > However I think this change is not sufficient. Each call to > pcie_config_aspm_link(link, policy_to_aspm_state(link)), e.g. in path > pcie_aspm_pm_state_change -> pcie_config_aspm_path will reset the > enabled states to the policy. That's right, let me work this in v2. Kai-Heng > >> mutex_unlock(_lock); >> up_read(_bus_sem); >> >
Re: [Regression] Can only do S3 once after "tpm: take TPM chip power gating out of tpm_transmit()"
> On Dec 8, 2020, at 18:17, Jarkko Sakkinen wrote:
>
> On Mon, Dec 07, 2020 at 12:42:53PM +0800, Kai-Heng Feng wrote:
>> Hi Jarkko,
>>
>> A user report that the system can only do S3 once. Subsequent S3 fails after
>> commit a3fbfae82b4c ("tpm: take TPM chip power gating out of
>> tpm_transmit()").
>>
>> Dmesg with the issue, collected under 5.10-rc2:
>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1891502/comments/14
>>
>> Dmesg without the issue, collected under 5.0.0-rc8:
>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1891502/comments/16
>>
>> Full bug report here:
>> https://bugs.launchpad.net/bugs/1891502
>>
>> Kai-Heng
>
> Relevant part:
>
>
> [80601.620149] tpm tpm0: Error (28) sending savestate before suspend
> [80601.620165] PM: __pnp_bus_suspend(): tpm_pm_suspend+0x0/0x90 returns 28
> [80601.620172] PM: dpm_run_callback(): pnp_bus_suspend+0x0/0x20 returns 28
> [80601.620178] PM: Device 00:01 failed to suspend: error 28
>
> Looking at this there are two issues:
>
> A. TPM_ORD_SAVESTATE command failing, this a new regression.
> B. When tpm_pm_suspend() fails, it should not fail the whole suspend
> procedure. And it returns the TPM error code back to the upper
> layers when it does so, which makes no sense. This is an old
> issue revealed by A.
>
> Let's look at tpm_pm_suspend():
>
> /*
> * We are about to suspend. Save the TPM state
> * so that it can be restored.
> */
> int tpm_pm_suspend(struct device *dev)
> {
> struct tpm_chip *chip = dev_get_drvdata(dev);
> int rc = 0;
>
> if (!chip)
> return -ENODEV;
>
> if (chip->flags & TPM_CHIP_FLAG_ALWAYS_POWERED)
> goto suspended;
>
> if ((chip->flags & TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED) &&
> !pm_suspend_via_firmware())
> goto suspended;
>
> if (!tpm_chip_start(chip)) {
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> tpm2_shutdown(chip, TPM2_SU_STATE);
> else
> rc = tpm1_pm_suspend(chip, tpm_suspend_pcr);
>
> tpm_chip_stop(chip);
> }
>
> suspended:
> return rc;
> }
> EXPORT_SYMBOL_GPL(tpm_pm_suspend);
>
> I would modify this into:
>
> /*
> * We are about to suspend. Save the TPM state
> * so that it can be restored.
> */
> int tpm_pm_suspend(struct device *dev)
> {
> struct tpm_chip *chip = dev_get_drvdata(dev);
> int rc = 0;
>
> if (!chip)
> return -ENODEV;
>
> if (chip->flags & TPM_CHIP_FLAG_ALWAYS_POWERED)
> goto suspended;
>
> if ((chip->flags & TPM_CHIP_FLAG_FIRMWARE_POWER_MANAGED) &&
> !pm_suspend_via_firmware())
> goto suspended;
>
> if (!tpm_chip_start(chip)) {
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> tpm2_shutdown(chip, TPM2_SU_STATE);
> else
> tpm1_pm_suspend(chip, tpm_suspend_pcr);
>
> tpm_chip_stop(chip);
> }
>
> suspended:
> return rc;
> }
> EXPORT_SYMBOL_GPL(tpm_pm_suspend);
>
> I.e. it's a good idea to put something into klog but that should not
> fail the whole suspend procedure. TPM is essentially opt-in feature.
>
> Of course issue A needs to be also sorted out but would this work as
> a quick initial fix? I can queue a patch for this. Is it possible to
> try out this fix for if I drop a patch?
Yes, possible test result from affected user.
I had to cut those code and do a diff side by side to find what changed.
Hopefully next time I can get one from `git diff`...
Kai-Heng
>
> /Jarkko
Re: [PATCH RFC 2/4] thermal/core: Add critical and hot ops
> On Dec 9, 2020, at 23:34, Daniel Lezcano wrote:
>
> Currently there is no way to the sensors to directly call an ops in
> interrupt mode without calling thermal_zone_device_update assuming all
> the trip points are defined.
>
> A sensor may want to do something special if a trip point is hot or
> critical.
>
> This patch adds the critical and hot ops to the thermal zone device,
> so a sensor can directly invoke them or let the thermal framework to
> call the sensor specific ones.
>
> Signed-off-by: Daniel Lezcano
Thanks. This can solve my issue once .critical callbacks are added in thermal
drivers.
Tested-by: Kai-Heng Feng
> ---
> drivers/thermal/thermal_core.c | 42 +-
> include/linux/thermal.h| 3 +++
> 2 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index afc02e7d1045..0366f3f076cc 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -375,6 +375,24 @@ static void thermal_emergency_poweroff(void)
> msecs_to_jiffies(poweroff_delay_ms));
> }
>
> +void thermal_zone_device_critical(struct thermal_zone_device *tz)
> +{
> + dev_emerg(>device, "%s: critical temperature reached, "
> + "shutting down\n", tz->type);
> +
> + mutex_lock(_lock);
> + if (!power_off_triggered) {
> + /*
> + * Queue a backup emergency shutdown in the event of
> + * orderly_poweroff failure
> + */
> + thermal_emergency_poweroff();
> + orderly_poweroff(true);
> + power_off_triggered = true;
> + }
> + mutex_unlock(_lock);
> +}
> +
> static void handle_critical_trips(struct thermal_zone_device *tz,
> int trip, enum thermal_trip_type trip_type)
> {
> @@ -391,22 +409,10 @@ static void handle_critical_trips(struct
> thermal_zone_device *tz,
> if (tz->ops->notify)
> tz->ops->notify(tz, trip, trip_type);
>
> - if (trip_type == THERMAL_TRIP_CRITICAL) {
> - dev_emerg(>device,
> - "critical temperature reached (%d C), shutting
> down\n",
> - tz->temperature / 1000);
> - mutex_lock(_lock);
> - if (!power_off_triggered) {
> - /*
> - * Queue a backup emergency shutdown in the event of
> - * orderly_poweroff failure
> - */
> - thermal_emergency_poweroff();
> - orderly_poweroff(true);
> - power_off_triggered = true;
> - }
> - mutex_unlock(_lock);
> - }
> + if (trip_type == THERMAL_TRIP_HOT && tz->ops->hot)
> + tz->ops->hot(tz);
> + else if (trip_type == THERMAL_TRIP_CRITICAL)
> + tz->ops->critical(tz);
> }
>
> static void handle_thermal_trip(struct thermal_zone_device *tz, int trip)
> @@ -1331,6 +1337,10 @@ thermal_zone_device_register(const char *type, int
> trips, int mask,
>
> tz->id = id;
> strlcpy(tz->type, type, sizeof(tz->type));
> +
> + if (!ops->critical)
> + ops->critical = thermal_zone_device_critical;
> +
> tz->ops = ops;
> tz->tzp = tzp;
> tz->device.class = _class;
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index f23a388ded15..125c8a4d52e6 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -79,6 +79,8 @@ struct thermal_zone_device_ops {
> enum thermal_trend *);
> int (*notify) (struct thermal_zone_device *, int,
> enum thermal_trip_type);
> + void (*hot)(struct thermal_zone_device *);
> + void (*critical)(struct thermal_zone_device *);
> };
>
> struct thermal_cooling_device_ops {
> @@ -399,6 +401,7 @@ void thermal_cdev_update(struct thermal_cooling_device *);
> void thermal_notify_framework(struct thermal_zone_device *, int);
> int thermal_zone_device_enable(struct thermal_zone_device *tz);
> int thermal_zone_device_disable(struct thermal_zone_device *tz);
> +void thermal_zone_device_critical(struct thermal_zone_device *tz);
> #else
> static inline struct thermal_zone_device *thermal_zone_device_register(
> const char *type, int trips, int mask, void *devdata,
> --
> 2.17.1
>
Re: [Nouveau] [PATCH v2] ALSA: hda: Continue to probe when codec probe fails
On Tue, Dec 22, 2020 at 1:56 AM Ilia Mirkin wrote:
>
> On Mon, Dec 21, 2020 at 11:33 AM Kai-Heng Feng
> wrote:
> >
> > [+Cc nouveau]
> >
> > On Fri, Dec 18, 2020 at 4:06 PM Takashi Iwai wrote:
> > [snip]
> > > > Quite possibly the system doesn't power up HDA controller when there's
> > > > no external monitor.
> > > > So when it's connected to external monitor, it's still needed for HDMI
> > > > audio.
> > > > Let me ask the user to confirm this.
> > >
> > > Yeah, it's the basic question whether the HD-audio is supposed to work
> > > on this machine at all. If yes, the current approach we take makes
> > > less sense - instead we should rather make the HD-audio controller
> > > working.
> >
> > Yea, confirmed that the Nvidia HDA works when HDMI is connected prior boot.
> >
> > > > > - The second problem is that pci_enable_device() ignores the error
> > > > > returned from pci_set_power_state() if it's -EIO. And the
> > > > > inaccessible access error returns -EIO, although it's rather a fatal
> > > > > problem. So the driver believes as the PCI device gets enabled
> > > > > properly.
> > > >
> > > > This was introduced in 2005, by Alan's 11f3859b1e85 ("[PATCH] PCI: Fix
> > > > regression in pci_enable_device_bars") to fix UHCI controller.
> > > >
> > > > >
> > > > > - The third problem is that HD-audio driver blindly believes the
> > > > > codec_mask read from the register even if it's a read failure as I
> > > > > already showed.
> > > >
> > > > This approach has least regression risk.
> > >
> > > Yes, but it assumes that HD-audio is really non-existent.
> >
> > I really don't know any good approach to address this.
> > On Windows, HDA PCI is "hidden" until HDMI cable is plugged, then the
> > driver will flag the magic bit to make HDA audio appear on the PCI
> > bus.
> > IIRC the current approach is to make nouveau and device link work.
>
> I don't have the full context of this discussion, but the kernel
> force-enables the HDA subfunction nowadays, irrespective of nouveau or
> nvidia or whatever:
That's the problem.
The nvidia HDA controller on the affected system only gets its power
after HDMI cable plugged, so the probe on boot fails.
Kai-Heng
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/quirks.c?h=v5.10#n5267
>
> Cheers,
>
> -ilia
Re: [PATCH v2] ALSA: hda: Continue to probe when codec probe fails
[+Cc Bjorn, Alan and linux-pci]
On Thu, Dec 17, 2020 at 12:57 AM Takashi Iwai wrote:
>
> On Wed, 16 Dec 2020 17:22:17 +0100,
> Takashi Iwai wrote:
> >
> > On Wed, 16 Dec 2020 17:07:45 +0100,
> > Kai-Heng Feng wrote:
> > >
> > > On Wed, Dec 16, 2020 at 11:58 PM Takashi Iwai wrote:
> > > >
> > > > On Wed, 16 Dec 2020 16:50:20 +0100,
> > > > Kai-Heng Feng wrote:
> > > > >
> > > > > On Wed, Dec 16, 2020 at 11:41 PM Takashi Iwai wrote:
> > > > > >
> > > > > > On Wed, 16 Dec 2020 13:47:24 +0100,
> > > > > > Kai-Heng Feng wrote:
> > > > > > >
> > > > > > > Similar to commit 9479e75fca37 ("ALSA: hda: Keep the controller
> > > > > > > initialization even if no codecs found"), when codec probe fails,
> > > > > > > it
> > > > > > > doesn't enable runtime suspend, and can prevent graphics card from
> > > > > > > getting powered down:
> > > > > > > [4.280991] snd_hda_intel :01:00.1: no codecs initialized
> > > > > > >
> > > > > > > $ cat /sys/bus/pci/devices/:01:00.1/power/runtime_status
> > > > > > > active
> > > > > > >
> > > > > > > So mark there's no codec and continue probing to let runtime PM
> > > > > > > to work.
> > > > > > >
> > > > > > > BugLink: https://bugs.launchpad.net/bugs/1907212
> > > > > > > Signed-off-by: Kai-Heng Feng
> > > > > >
> > > > > > Hm, but if the probe fails, doesn't it mean something really wrong?
> > > > > > IOW, how does this situation happen?
> > > > >
> > > > > The HDA controller is forcely created by quirk_nvidia_hda(). So
> > > > > probably there's really not an HDA controller.
> > > >
> > > > I still don't understand how non-zero codec_mask is passed.
> > > > The non-zero codec_mask means that BIOS or whatever believes that
> > > > HD-audio codecs are present and let HD-audio controller reporting the
> > > > presence. What error did you get at probing?
> > >
> > > [4.280991] snd_hda_intel :01:00.1: no codecs initialized
> > > Full dmesg here:
> > > https://launchpadlibrarian.net/510351476/dmesg.log
> >
> > The actual problems are shown before that line.
> >
> > [4.178848] pci :01:00.1: can't change power state from D3cold to D0
> > (config space inaccessible)
> > [4.179502] snd_hda_intel :01:00.1: can't change power state from
> > D3cold to D0 (config space inaccessible)
> > [4.179511] snd_hda_intel :01:00.1: can't change power state from
> > D3hot to D0 (config space inaccessible)
> >
> > [4.280571] hdaudio hdaudioC1D0: no AFG or MFG node found
> > [4.280633] hdaudio hdaudioC1D1: no AFG or MFG node found
> > [4.280685] hdaudio hdaudioC1D2: no AFG or MFG node found
> > [4.280736] hdaudio hdaudioC1D3: no AFG or MFG node found
> > [4.280788] hdaudio hdaudioC1D4: no AFG or MFG node found
> > [4.280839] hdaudio hdaudioC1D5: no AFG or MFG node found
> > [4.280892] hdaudio hdaudioC1D6: no AFG or MFG node found
> > [4.280943] hdaudio hdaudioC1D7: no AFG or MFG node found
> >
> > Could you check the codec_mask value read in
> > sound/hda/hdac_controller.c? I guess it reads 0xff.
> >
> > If that's the case, it can be corrected by the patch below.
> > But, we should check the cause of the first error (inaccessible config
> > space) in anyway; this must be the primary reason of the whole chain
> > of errors.
>
> Now I took a deeper look at the code. So we hit errors after errors:
> - The first problem is that quirk_nvidia_hda() enabled HD-audio even
> if it's non-functional by some reason. We may need additional
> checks there.
Quite possibly the system doesn't power up HDA controller when there's
no external monitor.
So when it's connected to external monitor, it's still needed for HDMI audio.
Let me ask the user to confirm this.
>
> - The second problem is that pci_enable_device() ignores the error
> returned from pci_set_power_state() if it's -EIO. And the
> inaccessible access error returns -EIO, although it's rather a fatal
> problem. So the driver believes as the PCI device gets enabled
> properly.
This was introduced in 2005, by Alan's 11f3859b1e85 ("[PATCH] PCI: Fix
regression in pci_enable_device_bars") to fix UHCI controller.
>
> - The third problem is that HD-audio driver blindly believes the
> codec_mask read from the register even if it's a read failure as I
> already showed.
This approach has least regression risk.
Kai-Heng
> Ideally we should address in the first place.
>
>
> Takashi
[PATCH 1/2] thermal: int340x: Add critical callback to override default shutdown behavior
We are seeing thermal shutdown on Intel based mobile workstations, the
shutdown happens during the first trip handle in
thermal_zone_device_register():
kernel: thermal thermal_zone15: critical temperature reached (101 C), shutting
down
However, we shouldn't do a thermal shutdown here, since
1) We may want to use a dedicated daemon, Intel's thermald in this case,
to handle thermal shutdown.
2) For ACPI based system, _CRT doesn't mean shutdown unless it's inside
ThermalZone namespace. ACPI Spec, 11.4.4 _CRT (Critical Temperature):
"... If this object it present under a device, the device’s driver
evaluates this object to determine the device’s critical cooling
temperature trip point. This value may then be used by the device’s
driver to program an internal device temperature sensor trip point."
So a "critical trip" here merely means we should take a more aggressive
cooling method.
As int340x device isn't present under ACPI ThermalZone, override the
default .critical callback to prevent surprising thermal shutdown.
Signed-off-by: Kai-Heng Feng
---
drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 6 ++
.../thermal/intel/int340x_thermal/int340x_thermal_zone.c| 6 ++
2 files changed, 12 insertions(+)
diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index 823354a1a91a..9778a6dba939 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -431,9 +431,15 @@ static int int3400_thermal_change_mode(struct
thermal_zone_device *thermal,
return result;
}
+static void int3400_thermal_critical(struct thermal_zone_device *thermal)
+{
+ dev_dbg(>device, "%s: critical temperature reached\n",
thermal->type);
+}
+
static struct thermal_zone_device_ops int3400_thermal_ops = {
.get_temp = int3400_thermal_get_temp,
.change_mode = int3400_thermal_change_mode,
+ .critical = int3400_thermal_critical,
};
static struct thermal_zone_params int3400_thermal_params = {
diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
index 6e479deff76b..d1248ba943a4 100644
--- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -146,12 +146,18 @@ static int int340x_thermal_get_trip_hyst(struct
thermal_zone_device *zone,
return 0;
}
+static void int340x_thermal_critical(struct thermal_zone_device *zone)
+{
+ dev_dbg(>device, "%s: critical temperature reached\n",
zone->type);
+}
+
static struct thermal_zone_device_ops int340x_thermal_zone_ops = {
.get_temp = int340x_thermal_get_zone_temp,
.get_trip_temp = int340x_thermal_get_trip_temp,
.get_trip_type = int340x_thermal_get_trip_type,
.set_trip_temp = int340x_thermal_set_trip_temp,
.get_trip_hyst = int340x_thermal_get_trip_hyst,
+ .critical = int340x_thermal_critical,
};
static int int340x_thermal_get_trip_config(acpi_handle handle, char *name,
--
2.29.2
[PATCH 2/2] thermal: intel_pch_thermal: Add critical callback to override default shutdown behavior
Like previous patch, the intel_pch_thermal device is not in ACPI
ThermalZone namespace, so a critical trip doesn't mean shutdown.
Override the default .critical callback to prevent surprising thermal
shutdoown.
Signed-off-by: Kai-Heng Feng
---
drivers/thermal/intel/intel_pch_thermal.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/thermal/intel/intel_pch_thermal.c
b/drivers/thermal/intel/intel_pch_thermal.c
index 41723c6c6c0c..527c91f5960b 100644
--- a/drivers/thermal/intel/intel_pch_thermal.c
+++ b/drivers/thermal/intel/intel_pch_thermal.c
@@ -326,10 +326,16 @@ static int pch_get_trip_temp(struct thermal_zone_device
*tzd, int trip, int *tem
return 0;
}
+static void pch_critical(struct thermal_zone_device *tzd)
+{
+ dev_dbg(>device, "%s: critical temperature reached\n", tzd->type);
+}
+
static struct thermal_zone_device_ops tzd_ops = {
.get_temp = pch_thermal_get_temp,
.get_trip_type = pch_get_trip_type,
.get_trip_temp = pch_get_trip_temp,
+ .critical = pch_critical,
};
enum board_ids {
--
2.29.2
Re: [PATCH 1/2] thermal: int340x: Add critical callback to override default shutdown behavior
On Mon, Dec 21, 2020 at 9:59 PM Daniel Lezcano
wrote:
>
> On 21/12/2020 14:52, Kai-Heng Feng wrote:
> > We are seeing thermal shutdown on Intel based mobile workstations, the
> > shutdown happens during the first trip handle in
> > thermal_zone_device_register():
> > kernel: thermal thermal_zone15: critical temperature reached (101 C),
> > shutting down
> >
> > However, we shouldn't do a thermal shutdown here, since
> > 1) We may want to use a dedicated daemon, Intel's thermald in this case,
> > to handle thermal shutdown.
> >
> > 2) For ACPI based system, _CRT doesn't mean shutdown unless it's inside
> > ThermalZone namespace. ACPI Spec, 11.4.4 _CRT (Critical Temperature):
> > "... If this object it present under a device, the device’s driver
> > evaluates this object to determine the device’s critical cooling
> > temperature trip point. This value may then be used by the device’s
> > driver to program an internal device temperature sensor trip point."
> >
> > So a "critical trip" here merely means we should take a more aggressive
> > cooling method.
> >
> > As int340x device isn't present under ACPI ThermalZone, override the
> > default .critical callback to prevent surprising thermal shutdown.
> >
> > Signed-off-by: Kai-Heng Feng
>
> I'll submit those changes for v5.11-rc1 and change the subject by:
>
> thermal: int340x: Fix unexpected shutdown at critical temperature
> thermal: pch: Fix unexpected shutdown at critical temperature
>
> Sounds good ?
Sounds good to me. Thanks!
Kai-Heng
>
> > ---
> > drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 6 ++
> > .../thermal/intel/int340x_thermal/int340x_thermal_zone.c| 6 ++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> > b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> > index 823354a1a91a..9778a6dba939 100644
> > --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> > +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> > @@ -431,9 +431,15 @@ static int int3400_thermal_change_mode(struct
> > thermal_zone_device *thermal,
> > return result;
> > }
> >
> > +static void int3400_thermal_critical(struct thermal_zone_device *thermal)
> > +{
> > + dev_dbg(>device, "%s: critical temperature reached\n",
> > thermal->type);
> > +}
> > +
> > static struct thermal_zone_device_ops int3400_thermal_ops = {
> > .get_temp = int3400_thermal_get_temp,
> > .change_mode = int3400_thermal_change_mode,
> > + .critical = int3400_thermal_critical,
> > };
> >
> > static struct thermal_zone_params int3400_thermal_params = {
> > diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> > b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> > index 6e479deff76b..d1248ba943a4 100644
> > --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> > +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> > @@ -146,12 +146,18 @@ static int int340x_thermal_get_trip_hyst(struct
> > thermal_zone_device *zone,
> > return 0;
> > }
> >
> > +static void int340x_thermal_critical(struct thermal_zone_device *zone)
> > +{
> > + dev_dbg(>device, "%s: critical temperature reached\n",
> > zone->type);
> > +}
> > +
> > static struct thermal_zone_device_ops int340x_thermal_zone_ops = {
> > .get_temp = int340x_thermal_get_zone_temp,
> > .get_trip_temp = int340x_thermal_get_trip_temp,
> > .get_trip_type = int340x_thermal_get_trip_type,
> > .set_trip_temp = int340x_thermal_set_trip_temp,
> > .get_trip_hyst = int340x_thermal_get_trip_hyst,
> > + .critical = int340x_thermal_critical,
> > };
> >
> > static int int340x_thermal_get_trip_config(acpi_handle handle, char *name,
> >
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
Re: [PATCH v2] ALSA: hda: Continue to probe when codec probe fails
[+Cc nouveau]
On Fri, Dec 18, 2020 at 4:06 PM Takashi Iwai wrote:
[snip]
> > Quite possibly the system doesn't power up HDA controller when there's
> > no external monitor.
> > So when it's connected to external monitor, it's still needed for HDMI
> > audio.
> > Let me ask the user to confirm this.
>
> Yeah, it's the basic question whether the HD-audio is supposed to work
> on this machine at all. If yes, the current approach we take makes
> less sense - instead we should rather make the HD-audio controller
> working.
Yea, confirmed that the Nvidia HDA works when HDMI is connected prior boot.
> > > - The second problem is that pci_enable_device() ignores the error
> > > returned from pci_set_power_state() if it's -EIO. And the
> > > inaccessible access error returns -EIO, although it's rather a fatal
> > > problem. So the driver believes as the PCI device gets enabled
> > > properly.
> >
> > This was introduced in 2005, by Alan's 11f3859b1e85 ("[PATCH] PCI: Fix
> > regression in pci_enable_device_bars") to fix UHCI controller.
> >
> > >
> > > - The third problem is that HD-audio driver blindly believes the
> > > codec_mask read from the register even if it's a read failure as I
> > > already showed.
> >
> > This approach has least regression risk.
>
> Yes, but it assumes that HD-audio is really non-existent.
I really don't know any good approach to address this.
On Windows, HDA PCI is "hidden" until HDMI cable is plugged, then the
driver will flag the magic bit to make HDA audio appear on the PCI
bus.
IIRC the current approach is to make nouveau and device link work.
Kai-Heng
>
>
> thanks,
>
> Takashi
Re: [PATCH 1/2] thermal: int340x: Add critical callback to override default shutdown behavior
On Tue, Dec 22, 2020 at 12:55 AM Srinivas Pandruvada
wrote:
>
> On Mon, 2020-12-21 at 21:52 +0800, Kai-Heng Feng wrote:
> > We are seeing thermal shutdown on Intel based mobile workstations,
> > the
> > shutdown happens during the first trip handle in
> > thermal_zone_device_register():
> > kernel: thermal thermal_zone15: critical temperature reached (101 C),
> > shutting down
> >
> > However, we shouldn't do a thermal shutdown here, since
> > 1) We may want to use a dedicated daemon, Intel's thermald in this
> > case,
> > to handle thermal shutdown.
> >
> > 2) For ACPI based system, _CRT doesn't mean shutdown unless it's
> > inside
> > ThermalZone namespace. ACPI Spec, 11.4.4 _CRT (Critical Temperature):
> > "... If this object it present under a device, the device’s driver
> > evaluates this object to determine the device’s critical cooling
> > temperature trip point. This value may then be used by the device’s
> > driver to program an internal device temperature sensor trip point."
> >
> > So a "critical trip" here merely means we should take a more
> > aggressive
> > cooling method.
> >
> > As int340x device isn't present under ACPI ThermalZone, override the
> > default .critical callback to prevent surprising thermal shutdown.
> >
> > Signed-off-by: Kai-Heng Feng
> > ---
> > drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 6
> > ++
> > .../thermal/intel/int340x_thermal/int340x_thermal_zone.c| 6
> > ++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> > b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> > index 823354a1a91a..9778a6dba939 100644
> > --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> > +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> > @@ -431,9 +431,15 @@ static int int3400_thermal_change_mode(struct
> > thermal_zone_device *thermal,
> > return result;
> > }
> >
> > +static void int3400_thermal_critical(struct thermal_zone_device
> > *thermal)
> > +{
> > + dev_dbg(>device, "%s: critical temperature
> > reached\n", thermal->type);
> > +}
> > +
> > static struct thermal_zone_device_ops int3400_thermal_ops = {
> > .get_temp = int3400_thermal_get_temp,
> > .change_mode = int3400_thermal_change_mode,
> > + .critical = int3400_thermal_critical,
> > };
>
> You don't need for int3400 device. This is a fake sensor.
Thanks. Let me send a v2.
Kai-Heng
>
> >
> > static struct thermal_zone_params int3400_thermal_params = {
> > diff --git
> > a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> > b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> > index 6e479deff76b..d1248ba943a4 100644
> > --- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> > +++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
> > @@ -146,12 +146,18 @@ static int int340x_thermal_get_trip_hyst(struct
> > thermal_zone_device *zone,
> > return 0;
> > }
> >
> > +static void int340x_thermal_critical(struct thermal_zone_device
> > *zone)
> > +{
> > + dev_dbg(>device, "%s: critical temperature reached\n",
> > zone->type);
> > +}
> > +
> > static struct thermal_zone_device_ops int340x_thermal_zone_ops = {
> > .get_temp = int340x_thermal_get_zone_temp,
> > .get_trip_temp = int340x_thermal_get_trip_temp,
> > .get_trip_type = int340x_thermal_get_trip_type,
> > .set_trip_temp = int340x_thermal_set_trip_temp,
> > .get_trip_hyst = int340x_thermal_get_trip_hyst,
> > + .critical = int340x_thermal_critical,
> > };
> >
> > static int int340x_thermal_get_trip_config(acpi_handle handle, char
> > *name,
>
> Thanks,
> Srinivas
>
>
Re: [PATCH v2] ALSA: hda: Continue to probe when codec probe fails
On Tue, Dec 22, 2020 at 12:47 AM Takashi Iwai wrote: [snip] > But what happens if you plug the HDMI cable later and want to use the > HDMI audio? It won't work with your fix, right? No it won't. It's possible to fix from nouveau, but it's at the mercy of Nvidia to fix their proprietary driver, which many users use. Kai-Heng > > > Takashi
[PATCH v2 1/2] thermal: int340x: Fix unexpected shutdown at critical temperature
We are seeing thermal shutdown on Intel based mobile workstations, the
shutdown happens during the first trip handle in
thermal_zone_device_register():
kernel: thermal thermal_zone15: critical temperature reached (101 C), shutting
down
However, we shouldn't do a thermal shutdown here, since
1) We may want to use a dedicated daemon, Intel's thermald in this case,
to handle thermal shutdown.
2) For ACPI based system, _CRT doesn't mean shutdown unless it's inside
ThermalZone namespace. ACPI Spec, 11.4.4 _CRT (Critical Temperature):
"... If this object it present under a device, the device’s driver
evaluates this object to determine the device’s critical cooling
temperature trip point. This value may then be used by the device’s
driver to program an internal device temperature sensor trip point."
So a "critical trip" here merely means we should take a more aggressive
cooling method.
As int340x device isn't present under ACPI ThermalZone, override the
default .critical callback to prevent surprising thermal shutdown.
Signed-off-by: Kai-Heng Feng
---
v2:
- Amend subject.
- Remove int3400 device.
.../thermal/intel/int340x_thermal/int340x_thermal_zone.c| 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
index 6e479deff76b..d1248ba943a4 100644
--- a/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
+++ b/drivers/thermal/intel/int340x_thermal/int340x_thermal_zone.c
@@ -146,12 +146,18 @@ static int int340x_thermal_get_trip_hyst(struct
thermal_zone_device *zone,
return 0;
}
+static void int340x_thermal_critical(struct thermal_zone_device *zone)
+{
+ dev_dbg(>device, "%s: critical temperature reached\n",
zone->type);
+}
+
static struct thermal_zone_device_ops int340x_thermal_zone_ops = {
.get_temp = int340x_thermal_get_zone_temp,
.get_trip_temp = int340x_thermal_get_trip_temp,
.get_trip_type = int340x_thermal_get_trip_type,
.set_trip_temp = int340x_thermal_set_trip_temp,
.get_trip_hyst = int340x_thermal_get_trip_hyst,
+ .critical = int340x_thermal_critical,
};
static int int340x_thermal_get_trip_config(acpi_handle handle, char *name,
--
2.29.2
[PATCH v2 2/2] thermal: intel: pch: Fix unexpected shutdown at critical temperature
Like previous patch, the intel_pch_thermal device is not in ACPI
ThermalZone namespace, so a critical trip doesn't mean shutdown.
Override the default .critical callback to prevent surprising thermal
shutdoown.
Signed-off-by: Kai-Heng Feng
---
v2:
- Amend subject.
drivers/thermal/intel/intel_pch_thermal.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/thermal/intel/intel_pch_thermal.c
b/drivers/thermal/intel/intel_pch_thermal.c
index 41723c6c6c0c..527c91f5960b 100644
--- a/drivers/thermal/intel/intel_pch_thermal.c
+++ b/drivers/thermal/intel/intel_pch_thermal.c
@@ -326,10 +326,16 @@ static int pch_get_trip_temp(struct thermal_zone_device
*tzd, int trip, int *tem
return 0;
}
+static void pch_critical(struct thermal_zone_device *tzd)
+{
+ dev_dbg(>device, "%s: critical temperature reached\n", tzd->type);
+}
+
static struct thermal_zone_device_ops tzd_ops = {
.get_temp = pch_thermal_get_temp,
.get_trip_type = pch_get_trip_type,
.get_trip_temp = pch_get_trip_temp,
+ .critical = pch_critical,
};
enum board_ids {
--
2.29.2
Re: [PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
On Fri, Jan 1, 2021 at 4:07 PM Takashi Iwai wrote:
>
> On Thu, 31 Dec 2020 19:06:43 +0100,
> Kai-Heng Feng wrote:
> >
> > On Thu, Dec 31, 2020 at 6:55 PM Takashi Iwai wrote:
> > >
> > > On Tue, 29 Dec 2020 14:38:15 +0100,
> > > Kai-Heng Feng wrote:
> > > >
> > > > System takes a very long time to suspend after commit 215a22ed31a1
> > > > ("ALSA: hda: Refactor codec PM to use direct-complete optimization"):
> > > > [ 90.065964] PM: suspend entry (s2idle)
> > > > [ 90.067337] Filesystems sync: 0.001 seconds
> > > > [ 90.185758] Freezing user space processes ... (elapsed 0.002
> > > > seconds) done.
> > > > [ 90.188713] OOM killer disabled.
> > > > [ 90.188714] Freezing remaining freezable tasks ... (elapsed 0.001
> > > > seconds) done.
> > > > [ 90.190024] printk: Suspending console(s) (use no_console_suspend to
> > > > debug)
> > > > [ 90.904912] intel_pch_thermal :00:12.0: CPU-PCH is cool [49C],
> > > > continue to suspend
> > > > [ 321.262505] snd_hda_codec_realtek ehdaudio0D0: Unable to sync
> > > > register 0x2b8000. -5
> > > > [ 328.426919] snd_hda_codec_realtek ehdaudio0D0: Unable to sync
> > > > register 0x2b8000. -5
> > > > [ 329.490933] ACPI: EC: interrupt blocked
> > > >
> > > > That commit keeps codec suspended during the system suspend. However,
> > > > SOF driver's runtime resume, which is for system suspend, calls
> > > > hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
> > > > work uses snd_hda_power_up_pm() which tries to resume the codec in
> > > > system suspend path, hence blocking the suspend process.
> > > >
> > > > So only check jack status if it's not in system PM process.
> > >
> > > After your previous patch set, the legacy HDA does queue the
> > > jackpoll_work only if jackpoll_interval is set. I suppose rather the
> > > same rule would be applied?
> >
> > It's queued in hda_codec_pm_complete(), which happens at the end of PM
> > process.
> > This one is queued in the middle of PM suspend, so it's not the same here.
>
> But why do we need the jack status check explicitly there if
> hda_codec_pm_complete() already does it (via re-queuing the resume)?
Because hda_resume() is called for both system and runtime resume, but
hda_codec_pm_complete() only gets called for system resume. So we
still need to cover the runtime resume case.
However, we can use pm_request_resume() to wakeup the codec and do the
jack detection, instead of queueing jackpoll_work directly. I'll do
the change in v2.
Kai-Heng
>
> thanks,
>
> Takashi
>
> >
> > Kai-Heng
> >
> > >
> > >
> > > thanks,
> > >
> > > Takashi
> > >
> > > >
> > > > Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use
> > > > direct-complete optimization")
> > > > Signed-off-by: Kai-Heng Feng
> > > > ---
> > > > sound/soc/sof/intel/hda-dsp.c | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/sound/soc/sof/intel/hda-dsp.c
> > > > b/sound/soc/sof/intel/hda-dsp.c
> > > > index 7d00107cf3b2..1c5e05b88a90 100644
> > > > --- a/sound/soc/sof/intel/hda-dsp.c
> > > > +++ b/sound/soc/sof/intel/hda-dsp.c
> > > > @@ -685,7 +685,8 @@ static int hda_resume(struct snd_sof_dev *sdev,
> > > > bool runtime_resume)
> > > > /* check jack status */
> > > > if (runtime_resume) {
> > > > hda_codec_jack_wake_enable(sdev, false);
> > > > - hda_codec_jack_check(sdev);
> > > > + if (sdev->system_suspend_target == SOF_SUSPEND_NONE)
> > > > + hda_codec_jack_check(sdev);
> > > > }
> > > >
> > > > /* turn off the links that were off before suspend */
> > > > --
> > > > 2.29.2
> > > >
> >
[PATCH v2 3/3] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
System takes a very long time to suspend after commit 215a22ed31a1
("ALSA: hda: Refactor codec PM to use direct-complete optimization"):
[ 90.065964] PM: suspend entry (s2idle)
[ 90.067337] Filesystems sync: 0.001 seconds
[ 90.185758] Freezing user space processes ... (elapsed 0.002 seconds) done.
[ 90.188713] OOM killer disabled.
[ 90.188714] Freezing remaining freezable tasks ... (elapsed 0.001 seconds)
done.
[ 90.190024] printk: Suspending console(s) (use no_console_suspend to debug)
[ 90.904912] intel_pch_thermal :00:12.0: CPU-PCH is cool [49C], continue
to suspend
[ 321.262505] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register
0x2b8000. -5
[ 328.426919] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register
0x2b8000. -5
[ 329.490933] ACPI: EC: interrupt blocked
That commit keeps codec suspended during the system suspend. However,
SOF driver's runtime resume, which is for system suspend, calls
hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
work uses snd_hda_power_up_pm() which tries to resume the codec in
system suspend path, hence blocking the suspend process.
So only check jack status if it's not in system PM process.
Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete
optimization")
Signed-off-by: Kai-Heng Feng
---
v2:
No change.
sound/soc/sof/intel/hda-dsp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 7d00107cf3b2..1c5e05b88a90 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -685,7 +685,8 @@ static int hda_resume(struct snd_sof_dev *sdev, bool
runtime_resume)
/* check jack status */
if (runtime_resume) {
hda_codec_jack_wake_enable(sdev, false);
- hda_codec_jack_check(sdev);
+ if (sdev->system_suspend_target == SOF_SUSPEND_NONE)
+ hda_codec_jack_check(sdev);
}
/* turn off the links that were off before suspend */
--
2.29.2
[PATCH v2 1/3] ASoC: SOF: Intel: hda: Resume codec to do jack detection
Instead of queueing jackpoll_work, runtime resume the codec to let it
use different jack detection methods based on jackpoll_interval.
This matches SOF driver's behavior with commit a6e7d0a4bdb0 ("ALSA: hda:
fix jack detection with Realtek codecs when in D3"). Since SOF only uses
Realtek codec, we don't need any additional check for the resume.
Signed-off-by: Kai-Heng Feng
---
v2:
No change.
sound/soc/sof/intel/hda-codec.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
index 6875fa570c2c..df59c79cfdfc 100644
--- a/sound/soc/sof/intel/hda-codec.c
+++ b/sound/soc/sof/intel/hda-codec.c
@@ -93,8 +93,7 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev)
* has been recorded in STATESTS
*/
if (codec->jacktbl.used)
- schedule_delayed_work(>jackpoll_work,
- codec->jackpoll_interval);
+ pm_request_resume(>core.dev);
}
#else
void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev) {}
--
2.29.2
[PATCH v2 2/3] ASoC: SOF: Intel: hda: Modify existing helper to disable WAKEEN
Modify hda_codec_jack_wake_enable() to also support disable WAKEEN.
In addition, this patch also moves the WAKEEN disablement call out of
hda_codec_jack_check() into hda_codec_jack_wake_enable().
This is a preparation for next patch.
No functional change intended.
Signed-off-by: Kai-Heng Feng
---
v2:
Mention it moves the disabling part into another function.
sound/soc/sof/intel/hda-codec.c | 16 +++-
sound/soc/sof/intel/hda-dsp.c | 6 --
sound/soc/sof/intel/hda.h | 2 +-
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
index df59c79cfdfc..b7e9931ead57 100644
--- a/sound/soc/sof/intel/hda-codec.c
+++ b/sound/soc/sof/intel/hda-codec.c
@@ -63,16 +63,18 @@ static int hda_codec_load_module(struct hda_codec *codec)
}
/* enable controller wake up event for all codecs with jack connectors */
-void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
+void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable)
{
struct hda_bus *hbus = sof_to_hbus(sdev);
struct hdac_bus *bus = sof_to_bus(sdev);
struct hda_codec *codec;
unsigned int mask = 0;
- list_for_each_codec(codec, hbus)
- if (codec->jacktbl.used)
- mask |= BIT(codec->core.addr);
+ if (enable) {
+ list_for_each_codec(codec, hbus)
+ if (codec->jacktbl.used)
+ mask |= BIT(codec->core.addr);
+ }
snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, mask);
}
@@ -81,12 +83,8 @@ void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
void hda_codec_jack_check(struct snd_sof_dev *sdev)
{
struct hda_bus *hbus = sof_to_hbus(sdev);
- struct hdac_bus *bus = sof_to_bus(sdev);
struct hda_codec *codec;
- /* disable controller Wake Up event*/
- snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, 0);
-
list_for_each_codec(codec, hbus)
/*
* Wake up all jack-detecting codecs regardless whether an event
@@ -96,7 +94,7 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev)
pm_request_resume(>core.dev);
}
#else
-void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev) {}
+void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable) {}
void hda_codec_jack_check(struct snd_sof_dev *sdev) {}
#endif /* CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC */
EXPORT_SYMBOL_NS(hda_codec_jack_wake_enable, SND_SOC_SOF_HDA_AUDIO_CODEC);
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 2b001151fe37..7d00107cf3b2 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -617,7 +617,7 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool
runtime_suspend)
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
if (runtime_suspend)
- hda_codec_jack_wake_enable(sdev);
+ hda_codec_jack_wake_enable(sdev, true);
/* power down all hda link */
snd_hdac_ext_bus_link_power_down_all(bus);
@@ -683,8 +683,10 @@ static int hda_resume(struct snd_sof_dev *sdev, bool
runtime_resume)
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
/* check jack status */
- if (runtime_resume)
+ if (runtime_resume) {
+ hda_codec_jack_wake_enable(sdev, false);
hda_codec_jack_check(sdev);
+ }
/* turn off the links that were off before suspend */
list_for_each_entry(hlink, >hlink_list, list) {
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 9ec8ae0fd649..a3b6f3e9121c 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -650,7 +650,7 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct device
*dev);
*/
void hda_codec_probe_bus(struct snd_sof_dev *sdev,
bool hda_codec_use_common_hdmi);
-void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev);
+void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable);
void hda_codec_jack_check(struct snd_sof_dev *sdev);
#endif /* CONFIG_SND_SOC_SOF_HDA */
--
2.29.2
[PATCH] ALSA: hda: Enable runtime PM when codec probe fails
When codec probe fails, it doesn't enable runtime suspend, and can
prevent graphics card from getting powered down:
[4.280991] snd_hda_intel :01:00.1: no codecs initialized
$ cat /sys/bus/pci/devices/:01:00.1/power/runtime_status
active
So enable runtime PM when codec probe fails, to let graphics card be
able to runtime suspend again.
Merge azx_probe_continue() into azx_probe() and just let probe fail for
this case could be a better approach. However that's a much bigger task
so let's settle with a quirk workaround.
BugLink: https://bugs.launchpad.net/bugs/1907212
Signed-off-by: Kai-Heng Feng
---
sound/pci/hda/hda_intel.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 6852668f1bcb..3fd920069268 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2328,7 +2328,7 @@ static int azx_probe_continue(struct azx *chip)
if (bus->codec_mask) {
err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
if (err < 0)
- goto out_free;
+ goto out_enable_rpm;
}
#ifdef CONFIG_SND_HDA_PATCH_LOADER
@@ -2360,6 +2360,7 @@ static int azx_probe_continue(struct azx *chip)
set_default_power_save(chip);
+out_enable_rpm:
if (azx_has_pm_runtime(chip)) {
pm_runtime_use_autosuspend(>dev);
pm_runtime_allow(>dev);
--
2.29.2
Re: [PATCH 1/3] thermal: core: Add indication for userspace usage
On Tue, Dec 15, 2020 at 2:22 AM Matthew Garrett wrote: > > On Sun, Nov 29, 2020 at 9:36 PM Kai-Heng Feng > wrote: > > > > We are seeing thermal shutdown on Intel based mobile workstations, the > > shutdown happens during the first trip handle in > > thermal_zone_device_register(): > > kernel: thermal thermal_zone15: critical temperature reached (101 C), > > shutting down > > Is the temperature reported by the thermal zone actually correct here? > 101 C seems extremely excessive. According to ODM/OEM, it's correct. It's a short period when Intel Turbo Boost kicks in. Kai-Heng
[PATCH v2] ALSA: hda: Continue to probe when codec probe fails
Similar to commit 9479e75fca37 ("ALSA: hda: Keep the controller
initialization even if no codecs found"), when codec probe fails, it
doesn't enable runtime suspend, and can prevent graphics card from
getting powered down:
[4.280991] snd_hda_intel :01:00.1: no codecs initialized
$ cat /sys/bus/pci/devices/:01:00.1/power/runtime_status
active
So mark there's no codec and continue probing to let runtime PM to work.
BugLink: https://bugs.launchpad.net/bugs/1907212
Signed-off-by: Kai-Heng Feng
---
sound/pci/hda/hda_intel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 6852668f1bcb..872a703dee43 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2328,7 +2328,7 @@ static int azx_probe_continue(struct azx *chip)
if (bus->codec_mask) {
err = azx_probe_codecs(chip, azx_max_codecs[chip->driver_type]);
if (err < 0)
- goto out_free;
+ bus->codec_mask = 0;
}
#ifdef CONFIG_SND_HDA_PATCH_LOADER
--
2.29.2
Re: [PATCH v2] ALSA: hda: Continue to probe when codec probe fails
On Wed, Dec 16, 2020 at 11:41 PM Takashi Iwai wrote:
>
> On Wed, 16 Dec 2020 13:47:24 +0100,
> Kai-Heng Feng wrote:
> >
> > Similar to commit 9479e75fca37 ("ALSA: hda: Keep the controller
> > initialization even if no codecs found"), when codec probe fails, it
> > doesn't enable runtime suspend, and can prevent graphics card from
> > getting powered down:
> > [4.280991] snd_hda_intel :01:00.1: no codecs initialized
> >
> > $ cat /sys/bus/pci/devices/:01:00.1/power/runtime_status
> > active
> >
> > So mark there's no codec and continue probing to let runtime PM to work.
> >
> > BugLink: https://bugs.launchpad.net/bugs/1907212
> > Signed-off-by: Kai-Heng Feng
>
> Hm, but if the probe fails, doesn't it mean something really wrong?
> IOW, how does this situation happen?
The HDA controller is forcely created by quirk_nvidia_hda(). So
probably there's really not an HDA controller.
>
> The usual no-codec state is for the devices that have a bogus HD-audio
> bus remaining while codecs aren't hooked or disabled by BIOS. For
> that, it makes to leave the controller driver and let it idle. But if
> you get really an error, it's something to fix there, not to just
> ignore in general.
The best approach I can think of is to make current two steps probe
into one. So when probe fails, the driver won't bind to the device.
What's the reason behind the two steps approach?
Kai-Heng
>
>
> thanks,
>
> Takashi
>
> > ---
> > sound/pci/hda/hda_intel.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> > index 6852668f1bcb..872a703dee43 100644
> > --- a/sound/pci/hda/hda_intel.c
> > +++ b/sound/pci/hda/hda_intel.c
> > @@ -2328,7 +2328,7 @@ static int azx_probe_continue(struct azx *chip)
> > if (bus->codec_mask) {
> > err = azx_probe_codecs(chip,
> > azx_max_codecs[chip->driver_type]);
> > if (err < 0)
> > - goto out_free;
> > + bus->codec_mask = 0;
> > }
> >
> > #ifdef CONFIG_SND_HDA_PATCH_LOADER
> > --
> > 2.29.2
> >
Re: [PATCH v2] ALSA: hda: Continue to probe when codec probe fails
On Wed, Dec 16, 2020 at 11:58 PM Takashi Iwai wrote:
>
> On Wed, 16 Dec 2020 16:50:20 +0100,
> Kai-Heng Feng wrote:
> >
> > On Wed, Dec 16, 2020 at 11:41 PM Takashi Iwai wrote:
> > >
> > > On Wed, 16 Dec 2020 13:47:24 +0100,
> > > Kai-Heng Feng wrote:
> > > >
> > > > Similar to commit 9479e75fca37 ("ALSA: hda: Keep the controller
> > > > initialization even if no codecs found"), when codec probe fails, it
> > > > doesn't enable runtime suspend, and can prevent graphics card from
> > > > getting powered down:
> > > > [4.280991] snd_hda_intel :01:00.1: no codecs initialized
> > > >
> > > > $ cat /sys/bus/pci/devices/:01:00.1/power/runtime_status
> > > > active
> > > >
> > > > So mark there's no codec and continue probing to let runtime PM to work.
> > > >
> > > > BugLink: https://bugs.launchpad.net/bugs/1907212
> > > > Signed-off-by: Kai-Heng Feng
> > >
> > > Hm, but if the probe fails, doesn't it mean something really wrong?
> > > IOW, how does this situation happen?
> >
> > The HDA controller is forcely created by quirk_nvidia_hda(). So
> > probably there's really not an HDA controller.
>
> I still don't understand how non-zero codec_mask is passed.
> The non-zero codec_mask means that BIOS or whatever believes that
> HD-audio codecs are present and let HD-audio controller reporting the
> presence. What error did you get at probing?
[4.280991] snd_hda_intel :01:00.1: no codecs initialized
Full dmesg here:
https://launchpadlibrarian.net/510351476/dmesg.log
>
>
> > > The usual no-codec state is for the devices that have a bogus HD-audio
> > > bus remaining while codecs aren't hooked or disabled by BIOS. For
> > > that, it makes to leave the controller driver and let it idle. But if
> > > you get really an error, it's something to fix there, not to just
> > > ignore in general.
> >
> > The best approach I can think of is to make current two steps probe
> > into one. So when probe fails, the driver won't bind to the device.
> > What's the reason behind the two steps approach?
>
> It's a sort of must, as the module loading is involved with binding
> with the codecs, as well as (optionally) request_firmware()
> invocation.
Ok. I also tried to use device_release_driver(), but azx_remove()
calls "cancel_work_sync(>probe_work)" so there will be a
deadlock.
Kai-Heng
>
>
> Takashi
--
Kai-Heng
Re: Time to re-enable Runtime PM per default for PCI devcies?
On Sat, Dec 26, 2020 at 11:26 PM Heiner Kallweit wrote:
>
> On 17.11.2020 17:57, Rafael J. Wysocki wrote:
> > On Tue, Nov 17, 2020 at 5:38 PM Bjorn Helgaas wrote:
> >>
> >> [+to Rafael, author of the commit you mentioned,
> >> +cc Mika, Kai Heng, Lukas, linux-pm, linux-kernel]
> >>
> >> On Tue, Nov 17, 2020 at 04:56:09PM +0100, Heiner Kallweit wrote:
> >>> More than 10 yrs ago Runtime PM was disabled per default by bb910a7040
> >>> ("PCI/PM Runtime: Make runtime PM of PCI devices inactive by default").
> >>>
> >>> Reason given: "avoid breakage on systems where ACPI-based wake-up is
> >>> known to fail for some devices"
> >>> Unfortunately the commit message doesn't mention any affected devices
> >>> or systems.
> >
> > Even if it did that, it wouldn't have been a full list almost for sure.
> >
> > We had received multiple problem reports related to that, most likely
> > because the ACPI PM in BIOSes at that time was tailored for
> > system-wide PM transitions only.
> >
>
> To follow up on this discussion:
> We could call pm_runtime_forbid() conditionally, e.g. with the following
> condition. This would enable runtime pm per default for all non-ACPI
> systems, and it uses the BIOS date as an indicator for a hopefully
> not that broken ACPI implementation. However I could understand the
> argument that this looks a little hacky ..
>
> if (IS_ENABLED(CONFIG_ACPI) && dmi_get_bios_year() <= 2016)
dmi_get_bios_year() may not be a good indicator. Last time I used it
caused regression, because the value changed after BIOS update.
For example, my MacBook Pro is manufactured in 2011, but
dmi_get_bios_year() returns 2018 with latest BIOS.
Kai-Heng
>
>
>
> >>> With Runtime PM disabled e.g. the PHY on network devices may remain
> >>> powered up even with no cable plugged in, affecting battery lifetime
> >>> on mobile devices. Currently we have to rely on the respective distro
> >>> or user to enable Runtime PM via sysfs (echo auto > power/control).
> >>> Some devices work around this restriction by calling pm_runtime_allow
> >>> in their probe routine, even though that's not recommended by
> >>> https://www.kernel.org/doc/Documentation/power/pci.txt
> >>>
> >>> Disabling Runtime PM per default seems to be a big hammer, a quirk
> >>> for affected devices / systems may had been better. And we still
> >>> have the option to disable Runtime PM for selected devices via sysfs.
> >>>
> >>> So, to cut a long story short: Wouldn't it be time to remove this
> >>> restriction?
> >>
> >> I don't know the history of this, but maybe Rafael or the others can
> >> shed some light on it.
> >
> > The systems that had those problems 10 years ago would still have
> > them, but I expect there to be more systems where runtime PM can be
> > enabled by default for PCI devices without issues.
> >
>
[PATCH 1/2] ASoC: SOF: Intel: hda: Modify existing helper to disable WAKEEN
Modify hda_codec_jack_wake_enable() to also support disable WAKEEN.
This is a preparation for next patch.
No functional change intended.
Signed-off-by: Kai-Heng Feng
---
sound/soc/sof/intel/hda-codec.c | 16 +++-
sound/soc/sof/intel/hda-dsp.c | 6 --
sound/soc/sof/intel/hda.h | 2 +-
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c
index 6875fa570c2c..bc9ac4abdab5 100644
--- a/sound/soc/sof/intel/hda-codec.c
+++ b/sound/soc/sof/intel/hda-codec.c
@@ -63,16 +63,18 @@ static int hda_codec_load_module(struct hda_codec *codec)
}
/* enable controller wake up event for all codecs with jack connectors */
-void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
+void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable)
{
struct hda_bus *hbus = sof_to_hbus(sdev);
struct hdac_bus *bus = sof_to_bus(sdev);
struct hda_codec *codec;
unsigned int mask = 0;
- list_for_each_codec(codec, hbus)
- if (codec->jacktbl.used)
- mask |= BIT(codec->core.addr);
+ if (enable) {
+ list_for_each_codec(codec, hbus)
+ if (codec->jacktbl.used)
+ mask |= BIT(codec->core.addr);
+ }
snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, mask);
}
@@ -81,12 +83,8 @@ void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
void hda_codec_jack_check(struct snd_sof_dev *sdev)
{
struct hda_bus *hbus = sof_to_hbus(sdev);
- struct hdac_bus *bus = sof_to_bus(sdev);
struct hda_codec *codec;
- /* disable controller Wake Up event*/
- snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, 0);
-
list_for_each_codec(codec, hbus)
/*
* Wake up all jack-detecting codecs regardless whether an event
@@ -97,7 +95,7 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev)
codec->jackpoll_interval);
}
#else
-void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev) {}
+void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable) {}
void hda_codec_jack_check(struct snd_sof_dev *sdev) {}
#endif /* CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC */
EXPORT_SYMBOL_NS(hda_codec_jack_wake_enable, SND_SOC_SOF_HDA_AUDIO_CODEC);
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 2b001151fe37..7d00107cf3b2 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -617,7 +617,7 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool
runtime_suspend)
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
if (runtime_suspend)
- hda_codec_jack_wake_enable(sdev);
+ hda_codec_jack_wake_enable(sdev, true);
/* power down all hda link */
snd_hdac_ext_bus_link_power_down_all(bus);
@@ -683,8 +683,10 @@ static int hda_resume(struct snd_sof_dev *sdev, bool
runtime_resume)
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
/* check jack status */
- if (runtime_resume)
+ if (runtime_resume) {
+ hda_codec_jack_wake_enable(sdev, false);
hda_codec_jack_check(sdev);
+ }
/* turn off the links that were off before suspend */
list_for_each_entry(hlink, >hlink_list, list) {
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 9ec8ae0fd649..a3b6f3e9121c 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -650,7 +650,7 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct device
*dev);
*/
void hda_codec_probe_bus(struct snd_sof_dev *sdev,
bool hda_codec_use_common_hdmi);
-void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev);
+void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable);
void hda_codec_jack_check(struct snd_sof_dev *sdev);
#endif /* CONFIG_SND_SOC_SOF_HDA */
--
2.29.2
[PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
System takes a very long time to suspend after commit 215a22ed31a1
("ALSA: hda: Refactor codec PM to use direct-complete optimization"):
[ 90.065964] PM: suspend entry (s2idle)
[ 90.067337] Filesystems sync: 0.001 seconds
[ 90.185758] Freezing user space processes ... (elapsed 0.002 seconds) done.
[ 90.188713] OOM killer disabled.
[ 90.188714] Freezing remaining freezable tasks ... (elapsed 0.001 seconds)
done.
[ 90.190024] printk: Suspending console(s) (use no_console_suspend to debug)
[ 90.904912] intel_pch_thermal :00:12.0: CPU-PCH is cool [49C], continue
to suspend
[ 321.262505] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register
0x2b8000. -5
[ 328.426919] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register
0x2b8000. -5
[ 329.490933] ACPI: EC: interrupt blocked
That commit keeps codec suspended during the system suspend. However,
SOF driver's runtime resume, which is for system suspend, calls
hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
work uses snd_hda_power_up_pm() which tries to resume the codec in
system suspend path, hence blocking the suspend process.
So only check jack status if it's not in system PM process.
Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete
optimization")
Signed-off-by: Kai-Heng Feng
---
sound/soc/sof/intel/hda-dsp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 7d00107cf3b2..1c5e05b88a90 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -685,7 +685,8 @@ static int hda_resume(struct snd_sof_dev *sdev, bool
runtime_resume)
/* check jack status */
if (runtime_resume) {
hda_codec_jack_wake_enable(sdev, false);
- hda_codec_jack_check(sdev);
+ if (sdev->system_suspend_target == SOF_SUSPEND_NONE)
+ hda_codec_jack_check(sdev);
}
/* turn off the links that were off before suspend */
--
2.29.2
[PATCH] HID: multitouch: Enable multi-input for Synaptics pointstick/touchpad device
Pointstick and its left/right buttons on HP EliteBook 850 G7 need
multi-input quirk to work correctly.
Signed-off-by: Kai-Heng Feng
---
drivers/hid/hid-multitouch.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index d670bcd57bde..0743ef51d3b2 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -2054,6 +2054,10 @@ static const struct hid_device_id mt_devices[] = {
HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
USB_VENDOR_ID_SYNAPTICS, 0xce08) },
+ { .driver_data = MT_CLS_WIN_8_FORCE_MULTI_INPUT,
+ HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
+ USB_VENDOR_ID_SYNAPTICS, 0xce09) },
+
/* TopSeed panels */
{ .driver_data = MT_CLS_TOPSEED,
MT_USB_DEVICE(USB_VENDOR_ID_TOPSEED2,
--
2.29.2
[PATCH] ALSA: hda/realtek: Enable mute and micmute LED on HP EliteBook 850 G7
HP EliteBook 850 G7 uses the same GPIO pins as ALC285_FIXUP_HP_GPIO_LED
to enable mute and micmute LED. So apply the quirk to enable the LEDs.
Signed-off-by: Kai-Heng Feng
---
sound/pci/hda/patch_realtek.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index dde5ba209541..b12e1f083029 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -7959,6 +7959,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
SND_PCI_QUIRK(0x103c, 0x8497, "HP Envy x360",
ALC269_FIXUP_HP_MUTE_LED_MIC3),
SND_PCI_QUIRK(0x103c, 0x84e7, "HP Pavilion 15",
ALC269_FIXUP_HP_MUTE_LED_MIC3),
SND_PCI_QUIRK(0x103c, 0x869d, "HP", ALC236_FIXUP_HP_MUTE_LED),
+ SND_PCI_QUIRK(0x103c, 0x8724, "HP EliteBook 850 G7",
ALC285_FIXUP_HP_GPIO_LED),
SND_PCI_QUIRK(0x103c, 0x8729, "HP", ALC285_FIXUP_HP_GPIO_LED),
SND_PCI_QUIRK(0x103c, 0x8736, "HP", ALC285_FIXUP_HP_GPIO_AMP_INIT),
SND_PCI_QUIRK(0x103c, 0x8760, "HP", ALC285_FIXUP_HP_MUTE_LED),
--
2.29.2
[PATCH] PM: sleep: core: Resume suspended device if direct-complete is disabled
HDA controller can't be runtime-suspended after commit 215a22ed31a1
("ALSA: hda: Refactor codjc PM to use direct-complete optimization"),
which enables direct-complete for HDA codec.
The HDA codec driver doesn't expect direct-complete will be disabled
after it returns a positive value from prepare() callback. So freeze()
is called directly when it's runtime-suspended, breaks the balance of
its internal codec_powered counting.
So if a device is prepared for direct-complete but PM core breaks the
assumption, resume the device to keep PM operations balanced.
Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete
optimization")
Signed-off-by: Kai-Heng Feng
---
drivers/base/power/main.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 46793276598d..9c0e25a92ad0 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1849,6 +1849,10 @@ static int device_prepare(struct device *dev,
pm_message_t state)
(ret > 0 || dev->power.no_pm_callbacks) &&
!dev_pm_test_driver_flags(dev, DPM_FLAG_NO_DIRECT_COMPLETE);
spin_unlock_irq(>power.lock);
+
+ if (ret > 0 && !dev->power.direct_complete)
+ pm_runtime_resume(dev);
+
return 0;
}
--
2.29.2
Re: [PATCH 2/2] ASoC: SOF: Intel: hda: Avoid checking jack on system suspend
On Thu, Dec 31, 2020 at 6:55 PM Takashi Iwai wrote:
>
> On Tue, 29 Dec 2020 14:38:15 +0100,
> Kai-Heng Feng wrote:
> >
> > System takes a very long time to suspend after commit 215a22ed31a1
> > ("ALSA: hda: Refactor codec PM to use direct-complete optimization"):
> > [ 90.065964] PM: suspend entry (s2idle)
> > [ 90.067337] Filesystems sync: 0.001 seconds
> > [ 90.185758] Freezing user space processes ... (elapsed 0.002 seconds)
> > done.
> > [ 90.188713] OOM killer disabled.
> > [ 90.188714] Freezing remaining freezable tasks ... (elapsed 0.001
> > seconds) done.
> > [ 90.190024] printk: Suspending console(s) (use no_console_suspend to
> > debug)
> > [ 90.904912] intel_pch_thermal :00:12.0: CPU-PCH is cool [49C],
> > continue to suspend
> > [ 321.262505] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register
> > 0x2b8000. -5
> > [ 328.426919] snd_hda_codec_realtek ehdaudio0D0: Unable to sync register
> > 0x2b8000. -5
> > [ 329.490933] ACPI: EC: interrupt blocked
> >
> > That commit keeps codec suspended during the system suspend. However,
> > SOF driver's runtime resume, which is for system suspend, calls
> > hda_codec_jack_check() and schedules jackpoll_work. The jackpoll
> > work uses snd_hda_power_up_pm() which tries to resume the codec in
> > system suspend path, hence blocking the suspend process.
> >
> > So only check jack status if it's not in system PM process.
>
> After your previous patch set, the legacy HDA does queue the
> jackpoll_work only if jackpoll_interval is set. I suppose rather the
> same rule would be applied?
It's queued in hda_codec_pm_complete(), which happens at the end of PM process.
This one is queued in the middle of PM suspend, so it's not the same here.
Kai-Heng
>
>
> thanks,
>
> Takashi
>
> >
> > Fixes: 215a22ed31a1 ("ALSA: hda: Refactor codec PM to use direct-complete
> > optimization")
> > Signed-off-by: Kai-Heng Feng
> > ---
> > sound/soc/sof/intel/hda-dsp.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
> > index 7d00107cf3b2..1c5e05b88a90 100644
> > --- a/sound/soc/sof/intel/hda-dsp.c
> > +++ b/sound/soc/sof/intel/hda-dsp.c
> > @@ -685,7 +685,8 @@ static int hda_resume(struct snd_sof_dev *sdev, bool
> > runtime_resume)
> > /* check jack status */
> > if (runtime_resume) {
> > hda_codec_jack_wake_enable(sdev, false);
> > - hda_codec_jack_check(sdev);
> > + if (sdev->system_suspend_target == SOF_SUSPEND_NONE)
> > + hda_codec_jack_check(sdev);
> > }
> >
> > /* turn off the links that were off before suspend */
> > --
> > 2.29.2
> >
Re: [PATCH 1/2] ASoC: SOF: Intel: hda: Modify existing helper to disable WAKEEN
On Thu, Dec 31, 2020 at 6:52 PM Takashi Iwai wrote:
>
> On Tue, 29 Dec 2020 14:38:14 +0100,
> Kai-Heng Feng wrote:
> >
> > Modify hda_codec_jack_wake_enable() to also support disable WAKEEN.
> > This is a preparation for next patch.
> >
> > No functional change intended.
>
> Maybe it's better to mention that this patch moves the WAKEEN
> disablement call out of hda_codec_jack_check() into
> hda_codec_jack_wake_enable(), too.
Ok, will update the commit log in v2.
Kai-Heng
>
>
> thanks,
>
> Takashi
>
> >
> > Signed-off-by: Kai-Heng Feng
> > ---
> > sound/soc/sof/intel/hda-codec.c | 16 +++-
> > sound/soc/sof/intel/hda-dsp.c | 6 --
> > sound/soc/sof/intel/hda.h | 2 +-
> > 3 files changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/sound/soc/sof/intel/hda-codec.c
> > b/sound/soc/sof/intel/hda-codec.c
> > index 6875fa570c2c..bc9ac4abdab5 100644
> > --- a/sound/soc/sof/intel/hda-codec.c
> > +++ b/sound/soc/sof/intel/hda-codec.c
> > @@ -63,16 +63,18 @@ static int hda_codec_load_module(struct hda_codec
> > *codec)
> > }
> >
> > /* enable controller wake up event for all codecs with jack connectors */
> > -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
> > +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable)
> > {
> > struct hda_bus *hbus = sof_to_hbus(sdev);
> > struct hdac_bus *bus = sof_to_bus(sdev);
> > struct hda_codec *codec;
> > unsigned int mask = 0;
> >
> > - list_for_each_codec(codec, hbus)
> > - if (codec->jacktbl.used)
> > - mask |= BIT(codec->core.addr);
> > + if (enable) {
> > + list_for_each_codec(codec, hbus)
> > + if (codec->jacktbl.used)
> > + mask |= BIT(codec->core.addr);
> > + }
> >
> > snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, mask);
> > }
> > @@ -81,12 +83,8 @@ void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev)
> > void hda_codec_jack_check(struct snd_sof_dev *sdev)
> > {
> > struct hda_bus *hbus = sof_to_hbus(sdev);
> > - struct hdac_bus *bus = sof_to_bus(sdev);
> > struct hda_codec *codec;
> >
> > - /* disable controller Wake Up event*/
> > - snd_hdac_chip_updatew(bus, WAKEEN, STATESTS_INT_MASK, 0);
> > -
> > list_for_each_codec(codec, hbus)
> > /*
> >* Wake up all jack-detecting codecs regardless whether an
> > event
> > @@ -97,7 +95,7 @@ void hda_codec_jack_check(struct snd_sof_dev *sdev)
> > codec->jackpoll_interval);
> > }
> > #else
> > -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev) {}
> > +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable) {}
> > void hda_codec_jack_check(struct snd_sof_dev *sdev) {}
> > #endif /* CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC */
> > EXPORT_SYMBOL_NS(hda_codec_jack_wake_enable, SND_SOC_SOF_HDA_AUDIO_CODEC);
> > diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
> > index 2b001151fe37..7d00107cf3b2 100644
> > --- a/sound/soc/sof/intel/hda-dsp.c
> > +++ b/sound/soc/sof/intel/hda-dsp.c
> > @@ -617,7 +617,7 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool
> > runtime_suspend)
> >
> > #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
> > if (runtime_suspend)
> > - hda_codec_jack_wake_enable(sdev);
> > + hda_codec_jack_wake_enable(sdev, true);
> >
> > /* power down all hda link */
> > snd_hdac_ext_bus_link_power_down_all(bus);
> > @@ -683,8 +683,10 @@ static int hda_resume(struct snd_sof_dev *sdev, bool
> > runtime_resume)
> >
> > #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
> > /* check jack status */
> > - if (runtime_resume)
> > + if (runtime_resume) {
> > + hda_codec_jack_wake_enable(sdev, false);
> > hda_codec_jack_check(sdev);
> > + }
> >
> > /* turn off the links that were off before suspend */
> > list_for_each_entry(hlink, >hlink_list, list) {
> > diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
> > index 9ec8ae0fd649..a3b6f3e9121c 100644
> > --- a/sound/soc/sof/intel/hda.h
> > +++ b/sound/soc/sof/intel/hda.h
> > @@ -650,7 +650,7 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct
> > device *dev);
> > */
> > void hda_codec_probe_bus(struct snd_sof_dev *sdev,
> >bool hda_codec_use_common_hdmi);
> > -void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev);
> > +void hda_codec_jack_wake_enable(struct snd_sof_dev *sdev, bool enable);
> > void hda_codec_jack_check(struct snd_sof_dev *sdev);
> >
> > #endif /* CONFIG_SND_SOC_SOF_HDA */
> > --
> > 2.29.2
> >
[PATCH 2/4] ALSA: hda: Stop mangling PCI MSI
The code predates 2005, it should be unnecessary now as PCI core handles
MSI much better nowadays.
So stop PCI MSI mangling in suspend/resume callbacks.
Signed-off-by: Kai-Heng Feng
---
sound/pci/hda/hda_intel.c | 15 ---
1 file changed, 15 deletions(-)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 749b88090970..b4aa1dcf1aae 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -1022,13 +1022,11 @@ static int azx_suspend(struct device *dev)
{
struct snd_card *card = dev_get_drvdata(dev);
struct azx *chip;
- struct hdac_bus *bus;
if (!azx_is_pm_ready(card))
return 0;
chip = card->private_data;
- bus = azx_bus(chip);
snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
/* An ugly workaround: direct call of __azx_runtime_suspend() and
* __azx_runtime_resume() for old Intel platforms that suffer from
@@ -1038,14 +1036,6 @@ static int azx_suspend(struct device *dev)
__azx_runtime_suspend(chip);
else
pm_runtime_force_suspend(dev);
- if (bus->irq >= 0) {
- free_irq(bus->irq, chip);
- bus->irq = -1;
- chip->card->sync_irq = -1;
- }
-
- if (chip->msi)
- pci_disable_msi(chip->pci);
trace_azx_suspend(chip);
return 0;
@@ -1060,11 +1050,6 @@ static int azx_resume(struct device *dev)
return 0;
chip = card->private_data;
- if (chip->msi)
- if (pci_enable_msi(chip->pci) < 0)
- chip->msi = 0;
- if (azx_acquire_irq(chip, 1) < 0)
- return -EIO;
if (chip->driver_caps & AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP)
__azx_runtime_resume(chip, false);
--
2.17.1
[PATCH 4/4] ALSA: hda: Reinstate runtime_allow() for all hda controllers
The broken jack detection should be fixed by commit a6e7d0a4bdb0 ("ALSA:
hda: fix jack detection with Realtek codecs when in D3"), let's try
enabling runtime PM by default again.
Signed-off-by: Kai-Heng Feng
---
sound/pci/hda/hda_intel.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index 14d935d8805f..6e83d00e1a51 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2330,6 +2330,7 @@ static int azx_probe_continue(struct azx *chip)
if (azx_has_pm_runtime(chip)) {
pm_runtime_use_autosuspend(>dev);
+ pm_runtime_allow(>dev);
pm_runtime_put_autosuspend(>dev);
}
--
2.17.1
[PATCH 3/4] ALSA: hda: Refactor controller PM to use direct-complete optimization
Similar to codec, we can use direct-complete optimization to keep HDA
controller suspended if conditions are met.
For most integrated HDA controllers, direct-complete won't happen because
ACPI wakeup is enabled for runtime suspend, but wakeup needs to be
disabled for system suspend.
For most HDA controller in discrete graphics, they will stay suspended
through system PM.
Note that HDA controllers don't need to use a complete() callback.
Codecs will decide if resume is needed.
While at it, also remove AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP, as the
original bug commit a6630529aecb ("ALSA: hda: Workaround for spurious
wakeups on some Intel platforms") solves doesn't happen with this
patch.
Signed-off-by: Kai-Heng Feng
---
sound/pci/hda/hda_controller.h | 2 +-
sound/pci/hda/hda_intel.c | 39 --
2 files changed, 15 insertions(+), 26 deletions(-)
diff --git a/sound/pci/hda/hda_controller.h b/sound/pci/hda/hda_controller.h
index be63ead8161f..fe171685492d 100644
--- a/sound/pci/hda/hda_controller.h
+++ b/sound/pci/hda/hda_controller.h
@@ -41,7 +41,7 @@
/* 24 unused */
#define AZX_DCAPS_COUNT_LPIB_DELAY (1 << 25) /* Take LPIB as delay */
#define AZX_DCAPS_PM_RUNTIME (1 << 26) /* runtime PM support */
-#define AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP (1 << 27) /* Workaround for spurious
wakeups after suspend */
+/* 27 unused */
#define AZX_DCAPS_CORBRP_SELF_CLEAR (1 << 28) /* CORBRP clears itself after
reset */
#define AZX_DCAPS_NO_MSI64 (1 << 29) /* Stick to 32-bit MSIs */
#define AZX_DCAPS_SEPARATE_STREAM_TAG (1 << 30) /* capture and playback use
separate stream tag */
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index b4aa1dcf1aae..14d935d8805f 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -297,8 +297,7 @@ enum {
/* PCH for HSW/BDW; with runtime PM */
/* no i915 binding for this as HSW/BDW has another controller for HDMI */
#define AZX_DCAPS_INTEL_PCH \
- (AZX_DCAPS_INTEL_PCH_BASE | AZX_DCAPS_PM_RUNTIME |\
-AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP)
+ (AZX_DCAPS_INTEL_PCH_BASE | AZX_DCAPS_PM_RUNTIME)
/* HSW HDMI */
#define AZX_DCAPS_INTEL_HASWELL \
@@ -1018,6 +1017,11 @@ static void __azx_runtime_resume(struct azx *chip, bool
from_rt)
}
#ifdef CONFIG_PM_SLEEP
+static int azx_prepare(struct device *dev)
+{
+ return pm_runtime_suspended(dev);
+}
+
static int azx_suspend(struct device *dev)
{
struct snd_card *card = dev_get_drvdata(dev);
@@ -1028,14 +1032,7 @@ static int azx_suspend(struct device *dev)
chip = card->private_data;
snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
- /* An ugly workaround: direct call of __azx_runtime_suspend() and
-* __azx_runtime_resume() for old Intel platforms that suffer from
-* spurious wakeups after S3 suspend
-*/
- if (chip->driver_caps & AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP)
- __azx_runtime_suspend(chip);
- else
- pm_runtime_force_suspend(dev);
+ __azx_runtime_suspend(chip);
trace_azx_suspend(chip);
return 0;
@@ -1050,11 +1047,7 @@ static int azx_resume(struct device *dev)
return 0;
chip = card->private_data;
-
- if (chip->driver_caps & AZX_DCAPS_SUSPEND_SPURIOUS_WAKEUP)
- __azx_runtime_resume(chip, false);
- else
- pm_runtime_force_resume(dev);
+ __azx_runtime_resume(chip, false);
snd_power_change_state(card, SNDRV_CTL_POWER_D0);
trace_azx_resume(chip);
@@ -1103,10 +1096,8 @@ static int azx_runtime_suspend(struct device *dev)
chip = card->private_data;
/* enable controller wake up event */
- if (snd_power_get_state(card) == SNDRV_CTL_POWER_D0) {
- azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) |
- STATESTS_INT_MASK);
- }
+ azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) |
+ STATESTS_INT_MASK);
__azx_runtime_suspend(chip);
trace_azx_runtime_suspend(chip);
@@ -1117,18 +1108,15 @@ static int azx_runtime_resume(struct device *dev)
{
struct snd_card *card = dev_get_drvdata(dev);
struct azx *chip;
- bool from_rt = snd_power_get_state(card) == SNDRV_CTL_POWER_D0;
if (!azx_is_pm_ready(card))
return 0;
chip = card->private_data;
- __azx_runtime_resume(chip, from_rt);
+ __azx_runtime_resume(chip, true);
/* disable controller Wake Up event*/
- if (from_rt) {
- azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) &
- ~STATESTS_INT_MASK);
- }
+ azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) &
+ ~STATESTS_INT_MASK);
trace_azx_runtime_resume(chip);
return 0;
@@ -1160,6 +114
[PATCH 1/4] ALSA: hda: Refactor codec PM to use direct-complete optimization
Upon system resume, hda_codec_pm_resume() uses hda_codec_force_resume()
to resume the codec. However, pm_runtime_force_resume() won't really
resume the codec because of pm_runtime_need_not_resume() check.
Hence, hda_codec_force_resume() schedules a jackpoll work, which is to
really power up the codec.
Instead of doing that, we can use direct-complete to make the PM flow
more straightforward, and keep codec always suspended through system PM
flow if conditions are met.
On system suspend, PM core will decide what to do based on
hda_codec_pm_prepare():
- If codec is not runtime-suspended, PM core will suspend and resume the
device as normal.
- If codec is runtime-suspended, PM core will try to keep it suspended.
If it's still suspended after system resume, we use
hda_codec_pm_complete() to resume codec if it's needed.
Signed-off-by: Kai-Heng Feng
---
sound/pci/hda/hda_codec.c | 45 +--
1 file changed, 29 insertions(+), 16 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index a356c21edb90..c2a510cc81bb 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -2934,7 +2934,7 @@ static void hda_call_codec_resume(struct hda_codec *codec)
snd_hdac_leave_pm(>core);
}
-static int hda_codec_runtime_suspend(struct device *dev)
+static int hda_codec_suspend(struct device *dev)
{
struct hda_codec *codec = dev_to_hda_codec(dev);
unsigned int state;
@@ -2953,7 +2953,7 @@ static int hda_codec_runtime_suspend(struct device *dev)
return 0;
}
-static int hda_codec_runtime_resume(struct device *dev)
+static int hda_codec_resume(struct device *dev)
{
struct hda_codec *codec = dev_to_hda_codec(dev);
@@ -2967,57 +2967,70 @@ static int hda_codec_runtime_resume(struct device *dev)
pm_runtime_mark_last_busy(dev);
return 0;
}
+
+static int hda_codec_runtime_suspend(struct device *dev)
+{
+ return hda_codec_suspend(dev);
+}
+
+static int hda_codec_runtime_resume(struct device *dev)
+{
+ return hda_codec_resume(dev);
+}
+
#endif /* CONFIG_PM */
#ifdef CONFIG_PM_SLEEP
-static int hda_codec_force_resume(struct device *dev)
+static int hda_codec_pm_prepare(struct device *dev)
+{
+ return pm_runtime_suspended(dev);
+}
+
+static void hda_codec_pm_complete(struct device *dev)
{
struct hda_codec *codec = dev_to_hda_codec(dev);
- int ret;
- ret = pm_runtime_force_resume(dev);
- /* schedule jackpoll work for jack detection update */
- if (codec->jackpoll_interval ||
- (pm_runtime_suspended(dev) && hda_codec_need_resume(codec)))
- schedule_delayed_work(>jackpoll_work,
- codec->jackpoll_interval);
- return ret;
+ if (pm_runtime_suspended(dev) &&
+ (hda_codec_need_resume(codec) || codec->forced_resume))
+ pm_request_resume(dev);
}
static int hda_codec_pm_suspend(struct device *dev)
{
dev->power.power_state = PMSG_SUSPEND;
- return pm_runtime_force_suspend(dev);
+ return hda_codec_suspend(dev);
}
static int hda_codec_pm_resume(struct device *dev)
{
dev->power.power_state = PMSG_RESUME;
- return hda_codec_force_resume(dev);
+ return hda_codec_resume(dev);
}
static int hda_codec_pm_freeze(struct device *dev)
{
dev->power.power_state = PMSG_FREEZE;
- return pm_runtime_force_suspend(dev);
+ return hda_codec_suspend(dev);
}
static int hda_codec_pm_thaw(struct device *dev)
{
dev->power.power_state = PMSG_THAW;
- return hda_codec_force_resume(dev);
+ return hda_codec_resume(dev);
}
static int hda_codec_pm_restore(struct device *dev)
{
dev->power.power_state = PMSG_RESTORE;
- return hda_codec_force_resume(dev);
+ return hda_codec_resume(dev);
}
#endif /* CONFIG_PM_SLEEP */
/* referred in hda_bind.c */
const struct dev_pm_ops hda_codec_driver_pm = {
#ifdef CONFIG_PM_SLEEP
+ .prepare = hda_codec_pm_prepare,
+ .complete = hda_codec_pm_complete,
.suspend = hda_codec_pm_suspend,
.resume = hda_codec_pm_resume,
.freeze = hda_codec_pm_freeze,
--
2.17.1
Re: [PATCH 1/4] ALSA: hda: Refactor codec PM to use direct-complete optimization
> On Oct 23, 2020, at 19:32, Takashi Iwai wrote:
>
> On Fri, 23 Oct 2020 12:23:35 +0200,
> Kai-Heng Feng wrote:
>>
>> +static void hda_codec_pm_complete(struct device *dev)
>> {
>> struct hda_codec *codec = dev_to_hda_codec(dev);
>> -int ret;
>>
>> -ret = pm_runtime_force_resume(dev);
>> -/* schedule jackpoll work for jack detection update */
>> -if (codec->jackpoll_interval ||
>> -(pm_runtime_suspended(dev) && hda_codec_need_resume(codec)))
>> -schedule_delayed_work(>jackpoll_work,
>> - codec->jackpoll_interval);
>> -return ret;
>> +if (pm_runtime_suspended(dev) &&
>> +(hda_codec_need_resume(codec) || codec->forced_resume))
>> +pm_request_resume(dev);
>
> You shouldn't drop the check of codec->jackpoll_interval. If this
> field is set, the codec driver has to resume no matter what it was, so
> that the polling can start up again.
Ok, will address in v2.
Kai-Heng
>
>
> thanks,
>
> Takashi
