Re: [PATCH v3 19/27] thunderbolt: Add new Thunderbolt PCI IDs

[Lukas Wunner]

On Mon, Jun 05, 2017 at 04:33:34PM +0300, Mika Westerberg wrote:
> On Mon, Jun 05, 2017 at 03:55:07PM +0300, Mika Westerberg wrote:
> > > Yes, it's useful to have ICM-based Thunderbolt on Macs and personally
> > > I'm totally fine with making it the default for Macs which support it.
> > > (I can't speak for Andreas, obviously.)
> > > 
> > > However it would be great to give people the *choice* between ICM versus
> > > native mode, for at least two reasons:
> > > 
> > > (1) Native mode uses free software.  (I assume the ICM firmware remains
> > >     closed source.)
> > > 
> > > (2) Native mode allows more versatility, e.g. how PCI tunnels are set up
> > >     to chained devices: PCI fanout or PCI direct routing, see:
> > >     https://developer.apple.com/library/content/documentation/HardwareDrivers/Conceptual/ThunderboltDevGuide/Basics/Basics.html
> > > 
> > >     Apple supports traffic prioritization to enable audio over Thunderbolt
> > >     with higher accuracy / minimal skew, I assume their choice to use
> > >     native mode was largely motivated by being able to support specialized
> > >     applications like that which are difficult or perhaps impossible to
> > >     implement in firmware:
> > >     http://pdfpiw.uspto.gov/.piw?Docid=09015384
> > 
> > Right, but do we need to do it now before we have got any feedback from
> > users using Macs with Alpine Ridge and ICM? We can always add the module
> > parameter later if really needed.
> 
> It turns out that this is not too big change after all so I think it can
> be included in the ICM patch if you want.
> 
> Basically we would have module parameter:
> 
> 	cm=auto|native|icm
> 
> where auto is the default.
> 
> Does that work?

Yes, great, thanks!

And in "auto" mode, it's ICM on non-Macs and TBT3-Macs, and native for
TBT1/TBT2-Macs, right?

Some Falcon Ridge Macs support ICM as well (e.g. the MacBookPro11,4
and MacBookPro11,5 introduced 2015), but maybe not all of them.
This will have to be tested.  If all of them support it, we could
later default to ICM for those as well, or use DMI data to constrain
it to specific models.

Best regards,

Lukas