[v2,1/5] soundwire: Add compute_params callback
diff mbox series

Message ID 20190813083550.5877-2-srinivas.kandagatla@linaro.org
State New
Headers show
Series
  • soundwire: Add support to Qualcomm SoundWire master
Related show

Commit Message

Srinivas Kandagatla Aug. 13, 2019, 8:35 a.m. UTC
From: Vinod Koul <vkoul@kernel.org>

This callback allows masters to compute the bus parameters required.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 drivers/soundwire/stream.c    | 10 ++++++++++
 include/linux/soundwire/sdw.h |  2 ++
 2 files changed, 12 insertions(+)

Comments

Pierre-Louis Bossart Aug. 13, 2019, 2:34 p.m. UTC | #1
On 8/13/19 3:35 AM, Srinivas Kandagatla wrote:
> From: Vinod Koul <vkoul@kernel.org>
> 
> This callback allows masters to compute the bus parameters required.

This looks like a partial use of the patch ('soundwire: Add Intel 
resource management algorithm')? see comments below

> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>   drivers/soundwire/stream.c    | 10 ++++++++++
>   include/linux/soundwire/sdw.h |  2 ++
>   2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index a0476755a459..60bc2fe42928 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c
> @@ -1483,6 +1483,16 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
>   		bus->params.bandwidth += m_rt->stream->params.rate *
>   			m_rt->ch_count * m_rt->stream->params.bps;
>   
> +		/* Compute params */
> +		if (bus->compute_params) {
> +			ret = bus->compute_params(bus);
> +			if (ret < 0) {
> +				dev_err(bus->dev, "Compute params failed: %d",
> +					ret);
> +				return ret;
> +			}
> +		}
> +

This would need to be duplicated for deprepare (as was done in the Intel 
patch).

>   		/* Program params */
>   		ret = sdw_program_params(bus);
>   		if (ret < 0) {
> diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
> index bea46bd8b6ce..aac68e879fae 100644
> --- a/include/linux/soundwire/sdw.h
> +++ b/include/linux/soundwire/sdw.h
> @@ -718,6 +718,7 @@ struct sdw_master_ops {
>    * Bit set implies used number, bit clear implies unused number.
>    * @bus_lock: bus lock
>    * @msg_lock: message lock
> + * @compute_params: points to Bus resource management implementation
>    * @ops: Master callback ops
>    * @port_ops: Master port callback ops
>    * @params: Current bus parameters
> @@ -739,6 +740,7 @@ struct sdw_bus {
>   	DECLARE_BITMAP(assigned, SDW_MAX_DEVICES);
>   	struct mutex bus_lock;
>   	struct mutex msg_lock;
> +	int (*compute_params)(struct sdw_bus *bus);

not sure I understand how it's set? We have a default in the Intel patch.

>   	const struct sdw_master_ops *ops;
>   	const struct sdw_master_port_ops *port_ops;
>   	struct sdw_bus_params params;
>
Srinivas Kandagatla Aug. 13, 2019, 6:17 p.m. UTC | #2
On 13/08/2019 15:34, Pierre-Louis Bossart wrote:
> On 8/13/19 3:35 AM, Srinivas Kandagatla wrote:
>> From: Vinod Koul <vkoul@kernel.org>
>>
>> This callback allows masters to compute the bus parameters required.
> 
> This looks like a partial use of the patch ('soundwire: Add Intel 
> resource management algorithm')? see comments below
> 

Yes it duplicate indeed!

I will use that patch!

--srini
Pierre-Louis Bossart Aug. 13, 2019, 7:08 p.m. UTC | #3
On 8/13/19 1:17 PM, Srinivas Kandagatla wrote:
> 
> 
> On 13/08/2019 15:34, Pierre-Louis Bossart wrote:
>> On 8/13/19 3:35 AM, Srinivas Kandagatla wrote:
>>> From: Vinod Koul <vkoul@kernel.org>
>>>
>>> This callback allows masters to compute the bus parameters required.
>>
>> This looks like a partial use of the patch ('soundwire: Add Intel 
>> resource management algorithm')? see comments below
>>
> 
> Yes it duplicate indeed!
> 
> I will use that patch!

Actually please don't...
we found issues with the Intel allocation so I'd rather have the big 
Intel patch split into two parts, with callbacks+prepare/deprepare 
changes going in first. It'll be much faster/nicer for everyone.
Vinod Koul Aug. 14, 2019, 4:05 a.m. UTC | #4
On 13-08-19, 14:08, Pierre-Louis Bossart wrote:
> On 8/13/19 1:17 PM, Srinivas Kandagatla wrote:
> > 
> > 
> > On 13/08/2019 15:34, Pierre-Louis Bossart wrote:
> > > On 8/13/19 3:35 AM, Srinivas Kandagatla wrote:
> > > > From: Vinod Koul <vkoul@kernel.org>
> > > > 
> > > > This callback allows masters to compute the bus parameters required.
> > > 
> > > This looks like a partial use of the patch ('soundwire: Add Intel
> > > resource management algorithm')? see comments below
> > > 
> > 
> > Yes it duplicate indeed!
> > 
> > I will use that patch!
> 
> Actually please don't...
> we found issues with the Intel allocation so I'd rather have the big Intel
> patch split into two parts, with callbacks+prepare/deprepare changes going
> in first. It'll be much faster/nicer for everyone.

I was about to say that as well. I think lets take this as is and Intel
algo can be on top of this. That seems easier for everyone to sort
dependencies
Vinod Koul Sept. 4, 2019, 9:28 a.m. UTC | #5
On 13-08-19, 09:35, Srinivas Kandagatla wrote:
> From: Vinod Koul <vkoul@kernel.org>
> 
> This callback allows masters to compute the bus parameters required.

Applied this to help manage cross dependencies with various folks better, thanks
Pierre-Louis Bossart Sept. 4, 2019, 1:36 p.m. UTC | #6
On 9/4/19 4:28 AM, Vinod Koul wrote:
> On 13-08-19, 09:35, Srinivas Kandagatla wrote:
>> From: Vinod Koul <vkoul@kernel.org>
>>
>> This callback allows masters to compute the bus parameters required.
> 
> Applied this to help manage cross dependencies with various folks better, thanks

Meh. doesn't really help, see my earlier comment, it's missing a call to 
compute_params in deprepare() so not sure this is works...

Patch
diff mbox series

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index a0476755a459..60bc2fe42928 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1483,6 +1483,16 @@  static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
 		bus->params.bandwidth += m_rt->stream->params.rate *
 			m_rt->ch_count * m_rt->stream->params.bps;
 
+		/* Compute params */
+		if (bus->compute_params) {
+			ret = bus->compute_params(bus);
+			if (ret < 0) {
+				dev_err(bus->dev, "Compute params failed: %d",
+					ret);
+				return ret;
+			}
+		}
+
 		/* Program params */
 		ret = sdw_program_params(bus);
 		if (ret < 0) {
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index bea46bd8b6ce..aac68e879fae 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -718,6 +718,7 @@  struct sdw_master_ops {
  * Bit set implies used number, bit clear implies unused number.
  * @bus_lock: bus lock
  * @msg_lock: message lock
+ * @compute_params: points to Bus resource management implementation
  * @ops: Master callback ops
  * @port_ops: Master port callback ops
  * @params: Current bus parameters
@@ -739,6 +740,7 @@  struct sdw_bus {
 	DECLARE_BITMAP(assigned, SDW_MAX_DEVICES);
 	struct mutex bus_lock;
 	struct mutex msg_lock;
+	int (*compute_params)(struct sdw_bus *bus);
 	const struct sdw_master_ops *ops;
 	const struct sdw_master_port_ops *port_ops;
 	struct sdw_bus_params params;