[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4a00f294-7f2b-4e29-abd5-2f08d7e12f59@amd.com>
Date: Sat, 10 Jan 2026 11:30:17 +0530
From: "Mukunda,Vijendar" <vijendar.mukunda@....com>
To: Mario Limonciello <superm1@...nel.org>, vkoul@...nel.org
Cc: yung-chuan.liao@...ux.intel.com, pierre-louis.bossart@...ux.dev,
Sunil-kumar.Dommati@....com, venkataprasad.potturu@....com,
linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] soundwire: amd: refactor bandwidth calculaiton logic
On 09/01/26 23:10, Mario Limonciello wrote:
> On 12/22/25 1:48 AM, Vijendar Mukunda wrote:
>> For ACP6.3/7.0/7.1/7.2 platforms, amd SoundWire manager doesn't have
>> banked registers concept. For bandwidth calculation, need to use static
>> mapping for block offset calculation based on master port request.
>> Refactor bandwidth calculation logic to support 6Mhz bus clock frequency
>> with frame size as 50 x 10, 125 x 2 and 12Mhz bus clock frequency with
>> frame size as 50 x 10 based on static port block offset logic.
>>
>> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@....com>
>> ---
>
> You have a typo in the subject "calculaiton"
Will fix it
>
> One more minor comment below. With those fixed you can add to next version:
>
> Reviewed-by: Mario Limonciello (AMD) <superm1@...nel.org>
>
>> drivers/soundwire/amd_manager.c | 52 ++++++++++++++++++++++++++++---
>> include/linux/soundwire/sdw_amd.h | 4 +++
>> 2 files changed, 52 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
>> index ee3c37a5a48b..4e4fd4c31019 100644
>> --- a/drivers/soundwire/amd_manager.c
>> +++ b/drivers/soundwire/amd_manager.c
>> @@ -480,27 +480,66 @@ static u32 amd_sdw_read_ping_status(struct sdw_bus *bus)
>> static int amd_sdw_compute_params(struct sdw_bus *bus, struct
>> sdw_stream_runtime *stream)
>> {
>> + struct amd_sdw_manager *amd_manager = to_amd_sdw(bus);
>> struct sdw_transport_data t_data = {0};
>> struct sdw_master_runtime *m_rt;
>> struct sdw_port_runtime *p_rt;
>> struct sdw_bus_params *b_params = &bus->params;
>> int port_bo, hstart, hstop, sample_int;
>> - unsigned int rate, bps;
>> + unsigned int rate, bps, channels;
>> + int stream_slot_size, max_slots;
>> + static int next_offset[AMD_SDW_MAX_MANAGER_COUNT] = {0};
>> + unsigned int inst_id = amd_manager->instance;
>> port_bo = 0;
>> hstart = 1;
>> hstop = bus->params.col - 1;
>> t_data.hstop = hstop;
>> t_data.hstart = hstart;
>> + if (next_offset[inst_id] == 0)
>> + next_offset[inst_id] = 1;
>
> This seems like pointless check no?
> You initialized
>
Will fix it and post V2 version patch set.
> > + static int next_offset[AMD_SDW_MAX_MANAGER_COUNT] = {0};
>
> So all offsets will be 0. Instead this can just be:
>
> next_offset[inst_id] = 1;
>
>> list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
>> rate = m_rt->stream->params.rate;
>> bps = m_rt->stream->params.bps;
>> + channels = m_rt->stream->params.ch_count;
>> sample_int = (bus->params.curr_dr_freq / rate);
>> +
>> + /* Compute slots required for this stream dynamically */
>> + stream_slot_size = bps * channels;
>> +
>> list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
>> - port_bo = (p_rt->num * 64) + 1;
>> - dev_dbg(bus->dev, "p_rt->num=%d hstart=%d hstop=%d port_bo=%d\n",
>> - p_rt->num, hstart, hstop, port_bo);
>> + if (p_rt->num >= amd_manager->max_ports) {
>> + dev_err(bus->dev, "Port %d exceeds max ports %d\n",
>> + p_rt->num, amd_manager->max_ports);
>> + return -EINVAL;
>> + }
>> +
>> + /* Static mapping logic */
>> + if (!amd_manager->port_offset_map[p_rt->num]) {
>> + if (bus->params.curr_dr_freq == 12000000) {
>> + max_slots = bus->params.row * (bus->params.col - 1);
>> + if (next_offset[inst_id] + stream_slot_size <=
>> + (max_slots - 1)) {
>> + amd_manager->port_offset_map[p_rt->num] =
>> + next_offset[inst_id];
>> + next_offset[inst_id] += stream_slot_size;
>> + } else {
>> + dev_err(bus->dev,
>> + "No space for port %d\n", p_rt->num);
>> + return -ENOMEM;
>> + }
>> + } else {
>> + amd_manager->port_offset_map[p_rt->num] =
>> + (p_rt->num * 64) + 1;
>> + }
>> + }
>> + port_bo = amd_manager->port_offset_map[p_rt->num];
>> + dev_dbg(bus->dev,
>> + "Port=%d hstart=%d hstop=%d port_bo=%d slots=%d
>> max_ports=%d\n",
>> + p_rt->num, hstart, hstop, port_bo, stream_slot_size,
>> + amd_manager->max_ports);
>> +
>> sdw_fill_xport_params(&p_rt->transport_params, p_rt->num,
>> false, SDW_BLK_GRP_CNT_1, sample_int,
>> port_bo, port_bo >> 8, hstart, hstop,
>> @@ -1093,6 +1132,11 @@ static int amd_sdw_manager_probe(struct
>> platform_device *pdev)
>> default:
>> return -EINVAL;
>> }
>> + amd_manager->max_ports = amd_manager->num_dout_ports +
>> amd_manager->num_din_ports;
>> + amd_manager->port_offset_map = devm_kcalloc(dev, amd_manager->max_ports,
>> + sizeof(int), GFP_KERNEL);
>> + if (!amd_manager->port_offset_map)
>> + return -ENOMEM;
>> prop = &amd_manager->bus.prop;
>> prop->mclk_freq = AMD_SDW_BUS_BASE_FREQ;
>> diff --git a/include/linux/soundwire/sdw_amd.h
>> b/include/linux/soundwire/sdw_amd.h
>> index fe31773d5210..470360a2723c 100644
>> --- a/include/linux/soundwire/sdw_amd.h
>> +++ b/include/linux/soundwire/sdw_amd.h
>> @@ -66,8 +66,10 @@ struct sdw_amd_dai_runtime {
>> * @status: peripheral devices status array
>> * @num_din_ports: number of input ports
>> * @num_dout_ports: number of output ports
>> + * @max_ports: total number of input ports and output ports
>> * @cols_index: Column index in frame shape
>> * @rows_index: Rows index in frame shape
>> + * @port_offset_map: dynamic array to map port block offset
>> * @instance: SoundWire manager instance
>> * @quirks: SoundWire manager quirks
>> * @wake_en_mask: wake enable mask per SoundWire manager
>> @@ -92,10 +94,12 @@ struct amd_sdw_manager {
>> int num_din_ports;
>> int num_dout_ports;
>> + int max_ports;
>> int cols_index;
>> int rows_index;
>> + int *port_offset_map;
>> u32 instance;
>> u32 quirks;
>> u32 wake_en_mask;
>
Powered by blists - more mailing lists