Skip to content

Soundwire: intel: set DPM_FLAG_NEVER_SKIP flag is runtime pm if disabled#1599

Closed
bardliao wants to merge 68 commits intothesofproject:integration/soundwire-debug-fixesfrom
bardliao:sdw-set-DPM_FLAG_NEVER_SKIP
Closed

Soundwire: intel: set DPM_FLAG_NEVER_SKIP flag is runtime pm if disabled#1599
bardliao wants to merge 68 commits intothesofproject:integration/soundwire-debug-fixesfrom
bardliao:sdw-set-DPM_FLAG_NEVER_SKIP

Conversation

@bardliao
Copy link
Collaborator

@bardliao bardliao commented Dec 5, 2019

System suspend/resume callbacks are always needed if runtime pm is
disabled.

Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com

Fixes #1595

plbossart and others added 7 commits December 4, 2019 16:08
When a Slave device becomes synchronized with the bus, it may report
its presence in PING frames, as well as optionally asserting an
in-band PREQ signal.

The bus driver will detect a new Device0, start the enumeration
process and assign it a non-zero device number. The SoundWire
enumeration provides an arbitration to deal with multiple Slaves
reporting ATTACHED at the same time. The bus driver will also invoke
the driver .probe() callback associated with this device. The probe()
depends on the Linux device core, which handles the match operations
and may result in modules being loaded.

Once the non-zero device number is programmed, the Slave will report
its new status in PING frames and the Master hardware will typically
report this status change with an interrupt. At this point, the
.update_status() callback of the codec driver will be invoked (usually
from an interrupt thread or workqueue scheduled from the interrupt
thread).

The first race condition which can happen is between the .probe(),
which allocates the resources, and .update_status() where
initializations are typically handled. The .probe() is only called
once during the initial boot, while .update_status() will be called
for every bus hardware reset and if the Slave device loses
synchronization (an unlikely event but with non-zero probability).

The time difference between the end of the enumeration process and a
change of status reported by the hardware may be as small as one
SoundWire PING frame. The scheduling of the interrupt thread, which
invokes .update_status() is not deterministic, but can be small enough
to create a race condition. With a 48 kHz frame rate and ideal
scheduling cases, the .probe() may be pre-empted within double-digit
microseconds.

Since there is no guarantee that the .probe() completes by the time
.update_status() is invoked as a result of an interrupt, it's not
unusual for the .update_status() to rely on data structures that have
not been allocated yet, leading to kernel oopses.

This patch adds a probe_complete utility, which is used in the
sdw_update_slave_status() routine. The codec driver does not need to
do anything and can safely assume all resources are allocated in its
update_status() callback.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
When the Master starts the bus (be it during the initial boot or
system resume), it usually performs a HardReset to make sure
electrical levels are correct, then enables the control channel.

While the PM framework guarantees that the Slave devices will only
become 'active' once the Master completes the bus initialization,
there is still a risk of a race condition: the Slave enumeration is
handled in a separate interrupt thread triggered by hardware status
changes, so the Slave device may not be ready to accept commands when
the Slave driver tries to access the registers and restore settings in
its resume or pm_runtime_resume callbacks. In those cases, any
read/write commands from/to the Slave device will result in a timeout.

This patch adds an enumeration_complete structure. When the bus is
goes through a HardReset sequence and restarted, the Slave will be
marked as UNATTACHED, which will result in a call to
init_completion().

When the Slave reports its presence during PING frames as a non-zero
Device, the Master hardware will issue an interrupt and the bus driver
will invoke complete(). The order between init_completion()/complete()
is predictable since this is a Master-initiated transition.

The Slave driver may use wait_for_completion() in its resume callback.
When regmap is used, the Slave driver will typically set its regmap in
cache-only mode on suspend, then on resume block on
wait_for_completion(&enumeration_complete) to guarantee it is safe to
start read/write transactions. It may then exit the cache-only mode
and use a regmap_sync to restore settings. All these steps are
optional, their use completely depends on the Slave device
capabilities and how the Slave driver is implemented.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Slave drivers may have different ways of handling their settings, with
or without regmap.

During the integration of codec drivers, done in partnership between
Intel and Realtek, it became desirable to implement a predictable
order between low-level initializations performed in .update_status()
(invoked by an interrupt thread) and the settings restored in the
resume steps (invoked by the PM core).

This patch builds on the previous solution to wait for the Slave
device to be fully enumerated. The complete() in this case is signaled
not before the .update_status() is called, but after .update_status()
returns. Without this patch, the settings were not properly restored,
leading to timing-dependent 'no sound after resume' or 'no headset
detected after resume' bug reports.

Depending on how initialization is handled, a Slave device driver may
wait for enumeration_complete, or for initialization_complete, both
are valid synchronization points. They are initialized at the same
time, they only differ on when complete() is invoked.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
…nces

The Slave device initialization can be split in 4 different cases:

1. Master-initiated hardware reset, system suspend-resume and
pm_runtime based on clock-stop mode1. To avoid timeouts and a bad
audio experience, the Slave device resume operations need to wait for
the Slave device to be re-enumerated and its settings restored.

2. Exit from clock-stop mode0. In this case, the Slave device is
required to remain enumerated and its context preserved while the
clock is stopped, so no re-initialization or wait_for_completion() is
necessary.

3. Slave-initiated pm_runtime D3 transition. With the parent child
relationship, it is possible that a Slave device becomes 'suspended'
while its parent is still 'active' with the bus clock still
toggling. In this case, during the pm_runtime resume operation, there
is no need to wait for any settings to be restored.

4. Slave reset (sync loss or implementation-defined). In that case the
bus remains operational and the Slave device will be re-initialized
when it becomes ATTACHED again.

In previous patches, we suggested the use of wait_for_completion() to
deal with the case #1, but case #2 and #3 do not need any wait.

To account for those differences, this patch adds an unattach_request
field. The field is explicitly set by the Master for the case #1, and
if non-zero the Slave device shall wait on resume. In all other cases,
the Slave resume operations can proceed without wait.

The only request tracked so far is Master HardReset, but the request
is declared as a bit mask for future extensions (if needed). The
definition for this value is added in bus.h and does not need to be
exposed in sdw.h

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The current interfaces between ASoC and SoundWire are limited by the
platform_device infrastructure to an init() and exit() (mapped to the
platform driver.probe and .remove)

To help with the platform detection, machine driver selection and
management of power dependencies between DSP and SoundWire IP, the
ASoC side requires:

a) an ACPI scan helper, to report if any devices are exposed in the
DSDT tables, and if any links are disabled by the BIOS.

b) a probe helper that allocates the resources without actually
starting the bus.

c) a startup helper which does start the bus when all power
dependencies are settled.

d) an exit helper to free all resources

e) an interrupt_enable/disable helper, typically invoked after the
startup helper but also used in suspend routines.

This patch moves all required interfaces to sdw_intel.h, mainly to
allow SoundWire and ASoC parts to be merged separately once the header
files are shared between trees.

To avoid compilation issues, the conflicts in intel_init.c are blindly
removed. This would in theory prevent the code from working, but since
there are no users of the Intel Soundwire driver this has no
impact. Functionality will be restored when the removal of platform
devices is complete.

Support for SoundWire + SOF builds will only be provided once all the
required pieces are upstream.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
…erations

The SoundWire DAIs for Intel platform are created in
drivers/soundwire/intel.c, while the communication with the Intel DSP
is all controlled in soc/sof/intel

When the DAI status changes, a callback is used to bridge the gap
between the two subsystems.

The naming of the existing 'config_stream' callback does not map well
with any of ALSA/ASoC concepts. This patch renames it as
'params_stream' to be more self-explanatory.

A new 'free_stream' callback is added in case any resources allocated
in the 'params_stream' stage need to be released. In the SOF
implementation, this is used in the hw_free case to release the DMA
channels over IPC.

These two callbacks now rely on structures which expose the link_id
and alh_stream_id (required by the firmware IPC), instead of a list of
parameters. The 'void *' definitions are changed to use explicit
types, as suggested on alsa-devel during earlier reviews.

Signed-off-by: Rander Wang <rander.wang@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The existing use of 6 handlers is problematic in MSI mode. Update
headers so that all shared interrupts can be handled with a single
handler.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Copy link
Member

@plbossart plbossart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks promising but we need more clarity on the issue and the solution

pm_runtime_set_active(&md->dev);
pm_runtime_enable(&md->dev);
} else {
dev_pm_set_driver_flags(&md->dev, DPM_FLAG_NEVER_SKIP);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry @bardliao I would need more explanations here. I did not understand the reports in issue #1595 and I am concerned that there are very few users of this flag (see below), and if it's needed here we would also need it for the PCI part when we disable PM runtime.

That said, I am not sure what happens if we disable pm_runtime but there are pretend RT700 devices exposed in the DSDT tables, that are not physically present on the board and are never attached. It could very well be a corner case where this is required, but that would need to be made explicit in the commit message.

@ranj063 @kv2019i @lyakh FYI, if you are familiar with that flag please help on this one.

git grep DPM_FLAG_NEVER_SKIP
Documentation/driver-api/pm/devices.rst:        ``DPM_FLAG_NEVER_SKIP`` and ``DPM_FLAG_SMART_PREPARE`` driver power
Documentation/power/pci.rst:The DPM_FLAG_NEVER_SKIP flag prevents the PM core from using the direct-complete
drivers/base/power/main.c:              !dev_pm_test_driver_flags(dev, DPM_FLAG_NEVER_SKIP);
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:                dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NEVER_SKIP);
drivers/gpu/drm/i915/intel_runtime_pm.c:        dev_pm_set_driver_flags(kdev, DPM_FLAG_NEVER_SKIP);
drivers/gpu/drm/radeon/radeon_kms.c:            dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NEVER_SKIP);
drivers/misc/mei/pci-me.c:      dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
drivers/misc/mei/pci-txe.c:     dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
drivers/net/ethernet/intel/e1000e/netdev.c:     dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
drivers/net/ethernet/intel/igb/igb_main.c:      dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
drivers/pci/pcie/portdrv_pci.c: dev_pm_set_driver_flags(&dev->dev, DPM_FLAG_NEVER_SKIP |
include/linux/pm.h:#define DPM_FLAG_NEVER_SKIP          BIT(0)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart @bardliao the NEVER_SKIP flag comes into picture if we're using direct complete for system suspend aka SMART_SUSPEND. As far as I know, this feature never worked for us (we do not enable it today). So we always resume the SOF audio device before system suspend. Additionally, the placement of this flag setting is a bit questionable since you set NEVER_SKIP only when runtime PM is disabled

Copy link
Collaborator Author

@bardliao bardliao Dec 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of this PR is to make sure intel_suspend() will be invoked in suspend if the link is not runtime suspended. Looking at power/main.c. It will check if (dev->power.direct_complete) to decide whether device's suspend callback function will be invoked or not. The direct_complete flag is basically assigned at device_prepare().

At this point, all sdw-master's direct_complete flag are true for the no_pm_callbacks flag is true. And the direct_complete flag of those codecs which are attached are also true for they are runtime suspended. However, for those codecs which are not attached, their direct_complete flag are false for the no_pm_callbacks flag is false and they are not runtime suspended.

It means the unattached codecs' suspend callback function will be invoked and dpm_clear_superiors_direct_complete() will be called after that.
At this point, sdw-master's no_pm_callbacks flag will be set to false

That's the reason why intel_suspend() will be triggered if there is a unattached codec on the sdw link, but not if there is no unattached codec on the sdw link.

To ensure intel_suspend() will be triggered whenever it is not runtime suspended, we need to set direct_complete flag false. I think the easiest way is to set DPM_FLAG_NEVER_SKIP, but I will check how can we let dev->power.no_pm_callbacks = false.

	dev->power.direct_complete = state.event == PM_EVENT_SUSPEND &&
		((pm_runtime_suspended(dev) && ret > 0) ||
		 dev->power.no_pm_callbacks) &&
		!dev_pm_test_driver_flags(dev, DPM_FLAG_NEVER_SKIP);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bardliao in the issue #1595, the PCI device and the SoundWire master don't enable PM runtime. The Rt700 is not present so pm_runtime is not enabled either for the codec device.

can you clarify this part:

It means the unattached codecs' suspend callback function will be invoked and dpm_clear_superiors_direct_complete() will be called after that.
At this point, sdw-master's no_pm_callbacks flag will be set to false

That's the reason why intel_suspend() will be triggered if there is a unattached codec on the sdw link, but not if there is no unattached codec on the sdw link.

I have no idea what the no_pm_callbacks means, really difficult for me to follow the logic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bardliao in the issue #1595, the PCI device and the SoundWire master don't enable PM runtime. The Rt700 is not present so pm_runtime is not enabled either for the codec device.

Right, but tr700's system sleep pm is enabled.

can you clarify this part:

It means the unattached codecs' suspend callback function will be invoked and dpm_clear_superiors_direct_complete() will be called after that.
At this point, sdw-master's no_pm_callbacks flag will be set to false

That's the reason why intel_suspend() will be triggered if there is a unattached codec on the sdw link, but not if there is no unattached codec on the sdw link.

__device_suspend() will test dev->power.direct_complete to decide whether device's pm callback function need to run or not. rt700's power.direct_complete will be false since its pm runtime is disabled.
It means tr700's system sleep suspend callback function will be invoked and more important dpm_clear_superiors_direct_complete(dev) will be called and it will clear rt700's parent which is sdw-master's power.direct_complete flag. That's why sdw-master's pm callback function which is intel_suspend() will be triggered if rt700's driver is loaded but pm runtime is disabled.

I have no idea what the no_pm_callbacks means, really difficult for me to follow the logic.

To my understanding, it means that there is no pm callback functions if no_pm_callback is true.
So the logic is we can directly complete the suspend process if there is no pm callback function for a device.

void device_pm_check_callbacks(struct device *dev)
{
	spin_lock_irq(&dev->power.lock);
	dev->power.no_pm_callbacks =
		(!dev->bus || (pm_ops_is_empty(dev->bus->pm) &&
		 !dev->bus->suspend && !dev->bus->resume)) &&
		(!dev->class || pm_ops_is_empty(dev->class->pm)) &&
		(!dev->type || pm_ops_is_empty(dev->type->pm)) &&
		(!dev->pm_domain || pm_ops_is_empty(&dev->pm_domain->ops)) &&
		(!dev->driver || (pm_ops_is_empty(dev->driver->pm) &&
		 !dev->driver->suspend && !dev->driver->resume));
	spin_unlock_irq(&dev->power.lock);
}

Ideally, the no_pm_callback flag of sdw-master device should be false, but it is true so far.
device_pm_check_callbacks() will be triggered by device_register(). However, dev->driver is null at that moment, and device_pm_check_callbacks() will not be triggered again after that. So, the no_pm_callback flag of sdw-master device is false.

bardliao and others added 4 commits December 6, 2019 15:13
…read

In MSI mode, the use of separate handlers and threads for the Intel
IPC, stream and SoundWire shared interrupt leads to timeouts and lost
interrupts.

The solution is to merge all interrupt handling across all links with
a single thread function. The use of a linked list enables this thread
function to walk through all contexts and figure out which link needs
attention.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
In ClockStop mode, the PCI device will be notified of a wake, which
will be handled from an interrupt thread.

Signed-off-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Some of the Intel SoundWire SHIM registers contain fields for
different links. Without protection, the master drivers for the
different links will access these shared registers, leading to invalid
configurations and timeouts (specifically when changing CPA/SPA
power-related registers and polling for the changes to be applied).

A mutex is added to make sure all rmw access to those registers are
serialized.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Due to power rail dependencies, the SoundWire Master driver cannot
make decisions on its own when entering pm runtime suspend.

Add quirk mask for each link, so that the SOF parent driver can inform
the SoundWire master driver of the desired behavior:
a) leave clock on
b) power-off instead of clock stop
c) power-off if all devices cannot generate wakes
d) force bus reset on clock restart

Note that for now the interface with the SOF driver relies on a single
mask for all links. If needed, the interface might be modified at a
later point to provide more freedom. The code at the lower level does
not assume any commonality between links.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@plbossart plbossart force-pushed the integration/soundwire-debug-fixes branch from 834f990 to 303bcd6 Compare December 6, 2019 21:33
Add clearer references to sdw_slave_driver for internal macros

No change for sdw_driver and module_sdw_driver to avoid compatibility
issues with existing codec devices

No functionality change.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Since we want to introduce master devices, rename macro so that we
have consistency between slave and master device access, following the
Grey Bus example.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Align with previous renames and shorten macro

No functionality change

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Before we add master driver support, make sure there is no ambiguity
and no occurrences of sdw_drv_ functions.

No functionality change.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
There are too many fields called 'res' so add prefix to make it easier
to track what the structures are.

Pure rename, no functionality change

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Currently the bus does not have any explicit support for master
devices.

First add explicit support for sdw_slave_type and error checks if this type
is not set.

In follow-up patches we can add support for the sdw_md_type (md==Master
Device), following the Grey Bus example.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Currently the code deals with uevents at the bus level, but we only care
for Slave events

Suggested-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Since we want an explicit support for the SoundWire Master device, add
the definitions, following the Grey Bus example.

Unlike for the Slave device, we do not set a variable when dealing
with the master uevent.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Use sdw_master_device and driver instead of platform devices

To quote GregKH:

"Don't mess with a platform device unless you really have no other
possible choice. And even then, don't do it and try to do something
else. Platform devices are really abused, don't perpetuate it "

In addition, rather than a plain-vanilla init/exit, this patch
provides 3 steps in the initialization (ACPI scan, probe, startup)
which make it easier to verify support and allocate required resources
as early as possible, and conversely help make the startup
lighter-weight with only hardware register setup.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart and others added 23 commits December 6, 2019 15:33
The state machine and notes don't accurately explain or allow
transitions from STREAM_DEPREPARED and STREAM_DISABLED.

Add more explanations and allow for more transitions as a result of a
trigger_stop(), trigger_suspend() and prepare(), depending on the
ALSA/ASoC layer behavior defined by the INFO_RESUME and INFO_PAUSE
flags.

Also add basic checks to help debug inconsistent states and illegal
state machine transitions.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
We don't need to prepare the stream again if the stream is already
prepared.

sdw_prepare_stream() could be called multiple times without calling
sdw_deprepare_stream(). We call sdw_prepare_stream() in the prepare
dai ops and sdw_deprepare_stream() in the hw_free dai ops. If an xrun
happens, sdw_prepare_stream() will be called but
sdw_deprepare_stream() will not, which results in an imbalance and an
invalid total bandwidth.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
…transition

After a system suspend, the ALSA/ASoC core will invoke the .prepare()
callback and a TRIGGER_START when INFO_RESUME is not supported.

Likewise, when an underflow occurs, the .prepare callback will be invoked.

In both cases, the stream can be in DISABLED mode, and will transition
into the PREPARED mode. We however don't want the bus bandwidth to be
recomputed.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
…suming

The .prepare() callback is invoked for normal streaming, underflows or
during the system resume transition. In the latter case, the context
for the ALH PDIs is lost, and the DSP is not initialized properly
either, but the bus parameters don't need to be recomputed.

Conversely, when doing a regular .prepare() during an underflow, the
ALH/SHIM registers shall not be changed as the hardware cannot be
reprogrammed after the DMA started (hardware spec requirement).

This patch adds storage of PDI and hw_params in the DAI dma context,
and the difference between the types of .prepare() usages is handled
via a simple boolean, updated when suspending, and tested for in the
.prepare() case.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Add quirk and pm_runtime idle scheduling to let the Master suspend if
no Slaves become attached. This can happen when a link is not marked
as disabled and has devices exposed in the DSDT, if the power is
controlled by sideband means or the link includes a pluggable
connector.

Signed-off-by: Rander Wang <rander.wang@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Config only needs to be updated when setting MCP_Config, MCP_Control
and MCP_CmdCtrl to make these register setting effective. When updating
config in master, master will communicate with slave to update status.
Communication will be failed when masters and slaves are in clock stop
state, and this unnecessary config update makes interrupt setting failed.

Tested on Comet Lake with soundwire enabled

Signed-off-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
…sters

Some of the SHIM registers exposed fields that are link specific, and
in addition some of the power-related registers (SPA/CPA) take time to
be updated. Uncontrolled access leads to timeouts or errors.

Add a mutex at the controller level, shared by all links, so that all
accesses to such registers are serialized, and follow a pattern of
read-modify-write.

GitHub issue: thesofproject#1555
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The laptop doesn't use DMIC, but use SOF.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
When a Slave reports multiple status in the sticky bits, find the
latest configuration from the mirror of the PING frame status and
update the status directly.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
This algorithm computes bus parameters like clock frequency, frame
shape and port transport parameters based on active stream(s) running
on the bus.

Developers can also implement their own .compute_params() callback for
specific resource management algorithm, and set if before calling
sdw_add_bus_master()

Credits: this patch is based on an earlier internal contribution by
Vinod Koul, Sanyog Kale, Shreyas Nc and Hardik Shah. All hard-coded
values were removed from the initial contribution to use BIOS
information instead.

FIXME: remove checkpatch report
WARNING: Reusing the krealloc arg is almost always a bug
+			group->rates = krealloc(group->rates,

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Some Realtek codecs don't support grp_ctrl_valid. Set it to false
to prevent DPN_BlockCtrl2 reg write failed.

Signed-off-by: Bard Liao <bardliao@gmail.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
If the programming of the dev_number fails due to an IO error, a new
device_number will be assigned, resulting in a leak.

Make sure we only assign a device_number once per Slave device.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The current value for this field is too low and results in some
samples getting dropped and playback being faster than normal.

Increase the value to a safer value.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Somehow Intel folks were confused, the property is 2x what the mclk
frequency actually is, as checked with the actual bus frequency.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
useful for debug, but can be verbose so only enable if strictly needed.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Another Dell laptop uses SOF with CML platform

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
We will free params when we goto out in sdw_compute_port_params(). But
we can't free params if it is not allocated successfully.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Fix cppcheck warning:

drivers/soundwire/cadence_master.c:992:9: style: Variable 'offset' is
assigned a value that is never used. [unreadVariable]
 offset += stream->num_out;
        ^

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@plbossart plbossart force-pushed the integration/soundwire-debug-fixes branch from 303bcd6 to f22b344 Compare December 6, 2019 21:34
@mengdonglin mengdonglin changed the title Soundwire: intel: set DPM_FLAG_NEVER_SKIP flag is runtime pm is disabled Soundwire: intel: set DPM_FLAG_NEVER_SKIP flag is runtime pm if disabled Dec 8, 2019
System suspend/resume callbacks are always needed if runtime pm is
disabled.

Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
@bardliao bardliao force-pushed the sdw-set-DPM_FLAG_NEVER_SKIP branch from 253068e to ebaa779 Compare December 9, 2019 08:05
@bardliao
Copy link
Collaborator Author

bardliao commented Dec 9, 2019

replaced by #1607

@plbossart plbossart force-pushed the integration/soundwire-debug-fixes branch from f22b344 to fa47d1f Compare December 9, 2019 18:25
@bardliao bardliao closed this Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants