Re: [PATCH v2 0/9] Add DSP topology management for SKL

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

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

On Sat, Sep 19, 2015 at 09:56:12AM -0700, Mark Brown wrote:
> On Fri, Sep 11, 2015 at 12:45:00PM +0100, Mark Brown wrote:
> > On Thu, Sep 03, 2015 at 01:44:54PM +0530, Vinod Koul wrote:
> 
> > > I was wondering if you have any feedback on this series ?
> 
> > Please don't send content free pings (as I'm sure I've mentioned
> > before).  I've not looked at this yet because it's another large,
> > invasive patch series which you sent at the very end of the development
> > cycle.
> 
> So, I've now looked at these, though not fully.  To expand on the above
> a bit: these are big bits of very device specific code that's really
> hard to read and understand and when I do read it I'm seeing similar
> issues again and again both in terms of big picture stuff and the
> trivial details (including the inconsistencies in things like commenting
> style that I'm not specifically pulling up).  I'm really worried that
> I'm often seeing fragments of things that may or may not work well
> together.
> 
> These Intel devices not only have a lot more code than most devices do,
> they also get a lot more exposure than most devices do to end users so I
> actually have to try to make some effort to understand what's going on
> and can't skimp on review even though I feel like I am.  I'm really
> concerned about where we're heading here and how robustly the results
> are going to work.
> 
> This all really slows down review, both directly in requiring detailed
> checks of hard to follow the code and as a result of making almost all
> other code review seem much more productive.
> 
> As well as the low level code quality issues it would really help if
> rather than continually sending large blocks of functionality we were
> starting from something that was a minimally viable but functional
> system and then extending that.  That should be a lot easier to follow.

Thanks Mark for the review,

I wouldn't have pinged but had been more than few weeks so was wondering if
this got missed. I did realise you were travelling and merge window came
up, but still thought Mark's typical turnaound is 1-2 weeks so was expecting
something. I do understand this take time but no harm in checking if it is
beyond usual window one expects.

Now on the code due to inherent nature of HW (we have HDA + DSP + I2S!!!)
and SW scalability, we do need lots of code for basic audio.
Btw  _this_ patch series is the last one to get basic minimal audio out of SKL.
After this we will add machine and then features...

We did decide to use DPCM, DAPM and topology here to solve manging the DSP
using these frameworks. yes hard coding a simple mixer and two pipes could
have made code look easy to follow but then we would be redoing those bits
as we scale these to defferent designs.

And to provide what we are trying here I have attached the SKL Topology conf
file to give a picture of topology we are trying to build as a reference.

On code issues, I will fix the style and wrappers issues you raised and
other instances where this might be required.

Also where you feel something is missing please do ask. Sometimes things are
perceived intuitive for us as we are doing this for quire some time but
may not be so for fresh eyes :)

Thanks
-- 
~Vinod

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

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