On 18-06-18, 15:20, Shreyas NC wrote:
> On Thu, Jun 14, 2018 at 01:34:13PM -0500, Pierre-Louis Bossart wrote:
> >
> > >diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> > >index 0e8a2eb5..771c963 100644
> > >--- a/drivers/soundwire/stream.c
> > >+++ b/drivers/soundwire/stream.c
> > >@@ -638,6 +638,8 @@ static int sdw_bank_switch(struct sdw_bus *bus)
> > > if (!wr_msg)
> > > return -ENOMEM;
> > >+ bus->defer_msg.msg = wr_msg;
> > >+
> > > wbuf = kzalloc(sizeof(*wbuf), GFP_KERNEL);
> > > if (!wbuf) {
> > > ret = -ENOMEM;
> > >@@ -658,17 +660,23 @@ static int sdw_bank_switch(struct sdw_bus *bus)
> > > SDW_MSG_FLAG_WRITE, wbuf);
> > > wr_msg->ssp_sync = true;
> > >- ret = sdw_transfer(bus, wr_msg);
> > >+ if (bus->multi_link)
> > >+ ret = sdw_transfer_defer(bus, wr_msg, &bus->defer_msg);
> > >+ else
> > >+ ret = sdw_transfer(bus, wr_msg);
> > >+
> >
> > Re-reading this v3, I wonder if there is a small logic flaw here or an
> > intended behavior.
> >
> > the bus->multi_link field is a hardware capability, it provides information
> > on whether the hardware can support synchronized bank switches across
> > multiple Master interfaces. This is independent of the streams being
> > handled, it's a bus property - not a stream one.
Pierre,
You are right that multi-link is a hw capability BUT it is also a
property of a stream. I can envision machines where bus may have the
capability but the system configuration may have links being not clubbed
in one board and made a multi-link stream in another board :)
> > However the default/typical case is a stream handled by a single master, so
> > when the hardware is capable of handling multiple masters this does not mean
> > you should use the deferred transfer, the regular case would work just fine
> > for a regular stream. If your SSP rate is faster than the common sync signal
> > it might even be beneficial not to use the deferred transfer.
> >
> > in other words, should the code be written with something like
> > if (bus->multi_link && master_rt_count > 1)
> > with master_rt_count being determined based on the stream properties.
Yes that makes sense to me
> > Or was the intent that you always rely on the deferred mechanism, even if
> > the stream is not handled by multiple masters? In that case, do we even need
> > the regular bank switch?
>
> After discussing with folks here, I understand that intent was to have an
> unified flow for both Multi and Single instance cases. Yes, we can always
> use the sdw_transfer_defer() and remove the references to bus->multi_link
> possibly. So, I will fix this up and post v4.
Shreyas,
That sounds like a good plan to me
--
~Vinod
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
