soundwire: add BRA properties#5710
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for new MIPI SoundWire DisCo/BRA device properties so BPT/BRA transfers can respect per-device constraints (max payload per frame and block alignment).
Changes:
- Extend
struct sdw_slave_propwithbra_block_alignmentandbra_max_data_per_frame, and parse them from firmware properties. - Thread BRA constraints into Cadence BPT/BRA buffer-size calculations and Intel ACE2x BPT stream setup.
- Update
SDW_BPT_MSG_MAX_BYTESdefinition/comment to reference the DisCo spec limit.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| include/linux/soundwire/sdw.h | Adds BRA properties to slave props; changes SDW_BPT_MSG_MAX_BYTES definition/comment. |
| drivers/soundwire/mipi_disco.c | Reads the new BRA properties from device properties. |
| drivers/soundwire/intel_ace2x.c | Uses new props to select max bytes/frame and pass alignment into buffer sizing. |
| drivers/soundwire/cadence_master.h | Updates sdw_cdns_bpt_find_buffer_sizes() signature to accept alignment. |
| drivers/soundwire/cadence_master.c | Applies block-alignment when computing actual bytes/frame in BPT/BRA sizing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bc527d5 to
f013d39
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
drivers/soundwire/cadence_master.c:2181
- When bra_block_alignment is set, the later clamp
if (data_bytes < actual_bpt_bytes) actual_bpt_bytes = data_bytes;can re-introduce a per-frame payload length that is not a multiple of bra_block_alignment (e.g., a single short transfer or the last partial frame). This contradicts the new block-alignment requirement and can lead to invalid BRA frames on devices that enforce the alignment. Consider enforcing thatdata_bytes/remainder is a multiple of bra_block_alignment (return -EINVAL otherwise), or redesigning the sizing logic to keep an alignedactual_bpt_bytesand handle padding/trimming appropriately (read vs write).
*data_per_frame = actual_bpt_bytes;
if (data_bytes < actual_bpt_bytes)
actual_bpt_bytes = data_bytes;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
drivers/soundwire/cadence_master.c:2181
*data_per_frameis set beforeactual_bpt_bytesmay be reduced todata_bytes, so callers can observe adata_per_framelarger than the size actually used to computenum_frames/buffer sizes. Withbra_block_alignmentthis also means the lateractual_bpt_bytes = data_bytespath can bypass the alignment constraint for small transfers. Consider applying themin(data_bytes, ...)step (and any alignment enforcement/handling for the last frame) before assigning*data_per_frameso the returned frame size is consistent with subsequent calculations.
*data_per_frame = actual_bpt_bytes;
if (data_bytes < actual_bpt_bytes)
actual_bpt_bytes = data_bytes;
| device_property_read_u32(dev, "mipi-sdw-bra-mode-max-data-per-frame", | ||
| &prop->bra_max_data_per_frame); |
There was a problem hiding this comment.
We don't check the DisCo table value here. We will check it when it is used instated.
The header[0] bit definitions are: Header[0] bits 7 – 6: BRA_HeaderType Header[0] bits 5 – 2: BRA_DeviceAddress[3:0] Header[0] bit 1 BRA_Opcode 1 => Write, 0 => Read Header[0] bit 0 BRA_NumBytes[8] And the header[1] indicates the BRA_NumBytes[7:0]. The existing code doesn't handle BRA_NumBytes[8] therefore the maximum BRA number of a frame is limited to 255. Fixes: fe8a9cf ("soundwire: pass sdw_bpt_section to cdns BPT helpers") Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Add a property to struct sdw_slave_prop equivalent to the Disco property "mipi-sdw-bra-mode-block-alignment". The SoundWire Disco specification defines this as: "The data payload size for this BRA Mode shall be an integer multiple of the value of this Property." Change-Id: I27ab84b0ed0f236a5eae58600400a4c386132480 Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> Co-developed-by: Bard Liao <yung-chuan.liao@linux.intel.com> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
The data pre frame size should be a multiple of bra_block_alignment. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Get the mipi-sdw-bra-mode-max-data-per-frame property which indicates the maximum data payload size (in bytes per frame excluding header, CRC, and footer) for the BRA Mode. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
The optional property indicates the maximum data payload size for the BRA mode. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Add mipi-sdw-bra-mode-max-data-per-frame and mipi-sdw-bra-mode-block-alignment properties support