SoundWire: mipi-disco: add partial 2.1 support #4857
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
looks good to me @plbossart |
ranj063
previously approved these changes
Mar 13, 2024
bardliao
reviewed
Mar 14, 2024
bardliao
reviewed
Mar 14, 2024
|
LGTM |
There was a problem hiding this comment.
I think sdw_intel_scan_controller() function can be made as generic . Only delta we see max no of links. If we handle it correctly, then I think we can use the same function sdw_intel_scan_controller() instead of referring amd_sdw_scan_controller() for AMD platforms.
May be we can go with generic function keeping max links value same for Intel and AMD platforms. We can take care invalid channel mask value in amd_init.c file. |
bardliao
previously approved these changes
Mar 15, 2024
Not sure how to go about sharing, AMD and Intel have both vendor-specific properties. We'd need some sort of callback or have a routine where we read all properties |
Agreed. |
bardliao
reviewed
Mar 18, 2024
andy-shev
reviewed
Mar 18, 2024
plbossart
force-pushed
the
sdw/disco-2.1
branch
from
15df3d8 to
c9819d1
Compare
plbossart
requested review from
lgirdwood,
kv2019i,
dbaluta and
ujfalusi
as code owners
plbossart
requested review from
vijendarmukunda,
ranj063,
andy-shev and
bardliao
plbossart
commented
Apr 30, 2024
bardliao
reviewed
May 2, 2024
bardliao
previously approved these changes
May 2, 2024
bardliao
reviewed
May 2, 2024
andy-shev
reviewed
May 2, 2024
andy-shev
reviewed
May 13, 2024
andy-shev
reviewed
May 20, 2024
Remove unnecessary initialization and un-shadow return code. Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
…ler() Optimize a bit by using an intermediate 'fwnode' variable. Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
For some reason we used an array of one u8 when the specification requires a u32. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The DisCo for SoundWire 2.0 spec adds support for a new sdw-manager-list property. Add it in backwards-compatible mode with 'sdw-master-count', which assumed that all links between 0..count-1 exist. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
pahole suggestion: swap position of 'm_rt_count'
before: pahole -C sdw_stream_runtime drivers/soundwire/soundwire-bus.ko
struct sdw_stream_runtime {
const char * name; /* 0 8 */
struct sdw_stream_params params; /* 8 12 */
enum sdw_stream_state state; /* 20 4 */
enum sdw_stream_type type; /* 24 4 */
/* XXX 4 bytes hole, try to pack */
struct list_head master_list; /* 32 16 */
int m_rt_count; /* 48 4 */
/* size: 56, cachelines: 1, members: 6 */
/* sum members: 48, holes: 1, sum holes: 4 */
/* padding: 4 */
/* last cacheline: 56 bytes */
};
after: pahole --reorganize -C sdw_stream_runtime drivers/soundwire/soundwire-bus.ko
struct sdw_stream_runtime {
const char * name; /* 0 8 */
struct sdw_stream_params params; /* 8 12 */
enum sdw_stream_state state; /* 20 4 */
enum sdw_stream_type type; /* 24 4 */
int m_rt_count; /* 28 4 */
struct list_head master_list; /* 32 16 */
/* size: 48, cachelines: 1, members: 6 */
/* last cacheline: 48 bytes */
}; /* saved 8 bytes! */
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Make pahole happy by moving pointers and u64 first instead of interleaving them. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The sdw_bus structure has seen multiple additions over the years. It's
one of the most used structures in this subsystem, so there's merit in
reshuffling the members a bit with 'pahole' to reduce holes and
structures across cache lines.
before:
struct sdw_bus {
struct device * dev; /* 0 8 */
struct sdw_master_device * md; /* 8 8 */
int controller_id; /* 16 4 */
unsigned int link_id; /* 20 4 */
int id; /* 24 4 */
/* XXX 4 bytes hole, try to pack */
struct list_head slaves; /* 32 16 */
long unsigned int assigned[1]; /* 48 8 */
struct mutex bus_lock; /* 56 160 */
/* --- cacheline 3 boundary (192 bytes) was 24 bytes ago --- */
struct lock_class_key bus_lock_key; /* 216 16 */
struct mutex msg_lock; /* 232 160 */
/* --- cacheline 6 boundary (384 bytes) was 8 bytes ago --- */
struct lock_class_key msg_lock_key; /* 392 16 */
int (*compute_params)(struct sdw_bus *); /* 408 8 */
const struct sdw_master_ops * ops; /* 416 8 */
const struct sdw_master_port_ops * port_ops; /* 424 8 */
struct sdw_bus_params params; /* 432 36 */
/* XXX 4 bytes hole, try to pack */
/* --- cacheline 7 boundary (448 bytes) was 24 bytes ago --- */
struct sdw_master_prop prop; /* 472 72 */
/* XXX last struct has 6 bytes of padding */
/* --- cacheline 8 boundary (512 bytes) was 32 bytes ago --- */
void * vendor_specific_prop; /* 544 8 */
struct list_head m_rt_list; /* 552 16 */
struct dentry * debugfs; /* 568 8 */
/* --- cacheline 9 boundary (576 bytes) --- */
struct irq_chip irq_chip; /* 576 264 */
/* --- cacheline 13 boundary (832 bytes) was 8 bytes ago --- */
struct irq_domain * domain; /* 840 8 */
struct sdw_defer defer_msg; /* 848 112 */
/* --- cacheline 15 boundary (960 bytes) --- */
unsigned int clk_stop_timeout; /* 960 4 */
u32 bank_switch_timeout; /* 964 4 */
bool multi_link; /* 968 1 */
/* XXX 3 bytes hole, try to pack */
int hw_sync_min_links; /* 972 4 */
int stream_refcount; /* 976 4 */
/* size: 984, cachelines: 16, members: 27 */
/* sum members: 969, holes: 3, sum holes: 11 */
/* padding: 4 */
/* paddings: 1, sum paddings: 6 */
/* last cacheline: 24 bytes */
};
after:
struct sdw_bus {
struct device * dev; /* 0 8 */
struct sdw_master_device * md; /* 8 8 */
struct lock_class_key bus_lock_key; /* 16 16 */
struct mutex bus_lock; /* 32 160 */
/* --- cacheline 3 boundary (192 bytes) --- */
struct list_head slaves; /* 192 16 */
struct lock_class_key msg_lock_key; /* 208 16 */
struct mutex msg_lock; /* 224 160 */
/* --- cacheline 6 boundary (384 bytes) --- */
struct list_head m_rt_list; /* 384 16 */
struct sdw_defer defer_msg; /* 400 112 */
/* --- cacheline 8 boundary (512 bytes) --- */
struct sdw_bus_params params; /* 512 36 */
int stream_refcount; /* 548 4 */
const struct sdw_master_ops * ops; /* 552 8 */
const struct sdw_master_port_ops * port_ops; /* 560 8 */
struct sdw_master_prop prop; /* 568 72 */
/* XXX last struct has 6 bytes of padding */
/* --- cacheline 10 boundary (640 bytes) --- */
void * vendor_specific_prop; /* 640 8 */
int hw_sync_min_links; /* 648 4 */
int controller_id; /* 652 4 */
unsigned int link_id; /* 656 4 */
int id; /* 660 4 */
int (*compute_params)(struct sdw_bus *); /* 664 8 */
long unsigned int assigned[1]; /* 672 8 */
unsigned int clk_stop_timeout; /* 680 4 */
u32 bank_switch_timeout; /* 684 4 */
struct irq_chip irq_chip; /* 688 264 */
/* --- cacheline 14 boundary (896 bytes) was 56 bytes ago --- */
struct irq_domain * domain; /* 952 8 */
/* --- cacheline 15 boundary (960 bytes) --- */
struct dentry * debugfs; /* 960 8 */
bool multi_link; /* 968 1 */
/* size: 976, cachelines: 16, members: 27 */
/* padding: 7 */
/* paddings: 1, sum paddings: 6 */
/* last cacheline: 16 bytes */
};
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
move pointers first, and move booleans together.
before:
struct sdw_slave_prop {
u32 mipi_revision; /* 0 4 */
bool wake_capable; /* 4 1 */
bool test_mode_capable; /* 5 1 */
bool clk_stop_mode1; /* 6 1 */
bool simple_clk_stop_capable; /* 7 1 */
u32 clk_stop_timeout; /* 8 4 */
u32 ch_prep_timeout; /* 12 4 */
enum sdw_clk_stop_reset_behave reset_behave; /* 16 4 */
bool high_PHY_capable; /* 20 1 */
bool paging_support; /* 21 1 */
bool bank_delay_support; /* 22 1 */
/* XXX 1 byte hole, try to pack */
enum sdw_p15_behave p15_behave; /* 24 4 */
bool lane_control_support; /* 28 1 */
/* XXX 3 bytes hole, try to pack */
u32 master_count; /* 32 4 */
u32 source_ports; /* 36 4 */
u32 sink_ports; /* 40 4 */
/* XXX 4 bytes hole, try to pack */
struct sdw_dp0_prop * dp0_prop; /* 48 8 */
struct sdw_dpn_prop * src_dpn_prop; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
struct sdw_dpn_prop * sink_dpn_prop; /* 64 8 */
u8 scp_int1_mask; /* 72 1 */
/* XXX 3 bytes hole, try to pack */
u32 quirks; /* 76 4 */
bool clock_reg_supported; /* 80 1 */
bool use_domain_irq; /* 81 1 */
/* size: 88, cachelines: 2, members: 23 */
/* sum members: 71, holes: 4, sum holes: 11 */
/* padding: 6 */
/* last cacheline: 24 bytes */
};
after:
truct sdw_slave_prop {
struct sdw_dp0_prop * dp0_prop; /* 0 8 */
struct sdw_dpn_prop * src_dpn_prop; /* 8 8 */
struct sdw_dpn_prop * sink_dpn_prop; /* 16 8 */
u32 mipi_revision; /* 24 4 */
bool wake_capable; /* 28 1 */
bool test_mode_capable; /* 29 1 */
bool clk_stop_mode1; /* 30 1 */
bool simple_clk_stop_capable; /* 31 1 */
u32 clk_stop_timeout; /* 32 4 */
u32 ch_prep_timeout; /* 36 4 */
enum sdw_clk_stop_reset_behave reset_behave; /* 40 4 */
bool high_PHY_capable; /* 44 1 */
bool paging_support; /* 45 1 */
bool bank_delay_support; /* 46 1 */
bool lane_control_support; /* 47 1 */
enum sdw_p15_behave p15_behave; /* 48 4 */
u32 master_count; /* 52 4 */
u32 source_ports; /* 56 4 */
u32 sink_ports; /* 60 4 */
/* --- cacheline 1 boundary (64 bytes) --- */
u32 quirks; /* 64 4 */
u8 scp_int1_mask; /* 68 1 */
bool clock_reg_supported; /* 69 1 */
bool use_domain_irq; /* 70 1 */
/* size: 72, cachelines: 2, members: 23 */
/* padding: 1 */
/* last cacheline: 8 bytes */
};
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Move pointers and booleans.
Before:
struct sdw_dp0_prop {
u32 max_word; /* 0 4 */
u32 min_word; /* 4 4 */
u32 num_words; /* 8 4 */
/* XXX 4 bytes hole, try to pack */
u32 * words; /* 16 8 */
bool BRA_flow_controlled; /* 24 1 */
bool simple_ch_prep_sm; /* 25 1 */
/* XXX 2 bytes hole, try to pack */
u32 ch_prep_timeout; /* 28 4 */
bool imp_def_interrupts; /* 32 1 */
/* size: 40, cachelines: 1, members: 8 */
/* sum members: 27, holes: 2, sum holes: 6 */
/* padding: 7 */
/* last cacheline: 40 bytes */
};
after:
struct sdw_dp0_prop {
u32 * words; /* 0 8 */
u32 max_word; /* 8 4 */
u32 min_word; /* 12 4 */
u32 num_words; /* 16 4 */
u32 ch_prep_timeout; /* 20 4 */
bool BRA_flow_controlled; /* 24 1 */
bool simple_ch_prep_sm; /* 25 1 */
bool imp_def_interrupts; /* 26 1 */
/* size: 32, cachelines: 1, members: 8 */
/* padding: 5 */
/* last cacheline: 32 bytes */
};
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
before:
struct sdw_dpn_prop {
u32 num; /* 0 4 */
u32 max_word; /* 4 4 */
u32 min_word; /* 8 4 */
u32 num_words; /* 12 4 */
u32 * words; /* 16 8 */
enum sdw_dpn_type type; /* 24 4 */
u32 max_grouping; /* 28 4 */
bool simple_ch_prep_sm; /* 32 1 */
/* XXX 3 bytes hole, try to pack */
u32 ch_prep_timeout; /* 36 4 */
u32 imp_def_interrupts; /* 40 4 */
u32 max_ch; /* 44 4 */
u32 min_ch; /* 48 4 */
u32 num_channels; /* 52 4 */
u32 * channels; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
u32 num_ch_combinations; /* 64 4 */
/* XXX 4 bytes hole, try to pack */
u32 * ch_combinations; /* 72 8 */
u32 modes; /* 80 4 */
u32 max_async_buffer; /* 84 4 */
bool block_pack_mode; /* 88 1 */
bool read_only_wordlength; /* 89 1 */
/* XXX 2 bytes hole, try to pack */
u32 port_encoding; /* 92 4 */
struct sdw_dpn_audio_mode * audio_modes; /* 96 8 */
/* size: 104, cachelines: 2, members: 22 */
/* sum members: 95, holes: 3, sum holes: 9 */
/* last cacheline: 40 bytes */
};
after:
struct sdw_dpn_prop {
struct sdw_dpn_audio_mode * audio_modes; /* 0 8 */
u32 num; /* 8 4 */
u32 max_word; /* 12 4 */
u32 min_word; /* 16 4 */
u32 num_words; /* 20 4 */
u32 * words; /* 24 8 */
enum sdw_dpn_type type; /* 32 4 */
u32 max_grouping; /* 36 4 */
u32 ch_prep_timeout; /* 40 4 */
u32 imp_def_interrupts; /* 44 4 */
u32 max_ch; /* 48 4 */
u32 min_ch; /* 52 4 */
u32 num_channels; /* 56 4 */
u32 num_ch_combinations; /* 60 4 */
/* --- cacheline 1 boundary (64 bytes) --- */
u32 * channels; /* 64 8 */
u32 * ch_combinations; /* 72 8 */
u32 modes; /* 80 4 */
u32 max_async_buffer; /* 84 4 */
u32 port_encoding; /* 88 4 */
bool block_pack_mode; /* 92 1 */
bool read_only_wordlength; /* 93 1 */
bool simple_ch_prep_sm; /* 94 1 */
/* size: 96, cachelines: 2, members: 22 */
/* padding: 1 */
/* last cacheline: 32 bytes */
};
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The concept of DPn audio-modes was never used by anyone, and was removed from the DisCo for SoundWire 2.0 specification. Remove the definitions and TODO. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The existing code assumes that there are no possible errors when using fwnode_property_read_u32_array(), because fwnode_property_count_u32() reads this array to determine its number of elements. We need to also protect the second read to be completely bullet-proof. Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The DisCo for SoundWire 2.0 spec adds support for the 'mipi-sdw-supported-clock-scales' property, which is just a rename. Add in a backwards-compatible manner. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The DisCo for SoundWire 2.0 spec renamed the 'mipi-sdw-slave-channelprepare-timeout', add support for the new definition in backwards-compatible ways. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The DisCo for SoundWire 2.0 spec adds support for a new property, but it's not very helpful. Add a comment to explain that it's intentionally ignored. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The DisCo for SoundWire 2.0 spec adds support for new 'mipi-sdw-sdca-interrupt-register-list' and 'mipi-sdw-commit-register-supported'. This patch only adds the definitions and property reads, but the use of these properties will come at some point in the future when needed. Note a slight conceptual disconnect between the MIPI DisCo definition of a boolean property and the Linux implementation. The latter only checks the presence of the property to set its value to 'true', whereas the MIPI definitions allow for a property with a 'false' value. This patch relies on a read_u8() even if the DisCo property is defined as boolean. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The SoundWire specification did not clearly require that ports could use all Lanes. Some SoundWire/SDCA peripheral adopters added restrictions on which lanes can be used by what port, and the DisCo for SoundWire 2.1 specification added a 'lane-list' property to model this hardware limitation. When not specified, the ports can use all Lanes. Otherwise, the 'lane-list' indicates which Lanes can be used, sorted by order of preference (most-preferred-first). This patch only reads the properties, the use of this property will come at a later time with multi-lane support. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart
force-pushed
the
sdw/disco-2.1
branch
from
aab5543 to
93a039c
Compare
|
new version: rebased, patches reordered and minor change suggested by @charleskeepax |
bardliao
approved these changes
Aug 13, 2024
|
All comments are addressed. Let's merge it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the changes needed to deal with the changes from DisCo 1.0 to 2.1 - without adding any SDCA entity parsing.
The property name changes are handled in a backwards-compatible way.
The most important part is the 'lane-list' property which does impact our multi-lane work, since not all lanes can be used for all ports.
@bardliao please double-check with the MIPI DisCo spec if I missed anything.
@ujfalusi @ranj063 comments welcome