Re: [PATCH 3/6] ASoC: Intel: Skylake: Add functions for DSP module configuration

[Vinod Koul <vinod.koul@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2044 bytes --]

On Wed, Jul 29, 2015 at 06:56:31PM +0100, Mark Brown wrote:
> On Wed, Jul 29, 2015 at 10:20:29PM +0530, Vinod Koul wrote:
> > On Wed, Jul 29, 2015 at 01:33:23PM +0100, Mark Brown wrote:
> 
> > > > +static int skl_alloc_queue(struct skl_module_pin *mpin,
> > > > +			struct skl_module_inst_id id, int max)
> > > > +{
> > > > +	struct skl_module_pin m_pin = skl_get_queue(mpin, id, max);
> > > > +
> > > > +	if (m_pin.pin_index < 0)
> > > > +		return -EINVAL;
> > > > +
> > > > +	if (m_pin.is_dynamic) {
> > > > +			m_pin.in_use = true;
> > > > +			m_pin.id.module_id = id.module_id;
> > > > +			m_pin.id.instance_id = id.instance_id;
> > > > +	}
> 
> > > Double indentation here.  It's really unclear to me why a static pin
> > > doesn't need to get merked as in use?
> 
> > Oops, will fix
> 
> > In static pin case, the pins come from topology binary where these are fixed
> > for a configuration, so don't need to be allocated thus marked and hence
> > freed.
> 
> But isn't this also protecting against attempts to use the resource
> multiple times within the configuration (or shouldn't we be doing that)?
In case of static since a module pin is allocated while designing
topology we shouldn't have clash as per design of topology

For example I have a Gain module connected to Mixer. Gain module pin 0 will
be allocated to connect to Mixer Pin0. I wont assign Pin0 to any other
module if I am doing static mapping.

Whereas in dynamic we will check for first free pin and allocate.

If all the pins has same meaning then dynamic would make sense, but non
linear modules need reference signals so they have special pins so we need
both approaches here

> Please bear in mind that this stuff has basically zero documentation or
> explanation so I'm kind of guessing as to what this is supposed to do.
I did try to add explanation where I felt was missing, but yes this is good
feedback, I will add more bits in next rev. Also please do point out where
you feel we missed.

Thanks
-- 
~Vinod

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]