Re: [PATCH v4 03/14] ASoC: hdac_hdmi: Use list to add pins and converters

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

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

On Fri, Jan 08, 2016 at 01:32:51PM +0000, Mark Brown wrote:
> On Wed, Dec 09, 2015 at 09:46:10PM +0530, Subhransu S. Prusty wrote:
> 
> > Future platforms may have a different set of pins/converters.
> > So use lists to add pins and converters based on enumeration.
> 
> > Also it may be required to connect any converter to any pin
> > dynamically as per different use cases (for example DP is
> > connected to pin 6 on skylake board). So this will help in
> > dynamically select and route.
> 
> This sounds like it's supposed to be supporting multiple outputs but...
> 
> > +	/*
> > +	 * Currently on board only 1 pin and 1 converter is enabled for
> > +	 * simplification, more will be added eventually
> > +	 * So using fixed map for dai_id:pin:cvt
> > +	 */
> > +	cvt = list_first_entry(&hdmi->cvt_list, struct hdac_hdmi_cvt, head);
> > +	pin = list_first_entry(&hdmi->pin_list, struct hdac_hdmi_pin, head);
> 
> ...it looks like we still only support one random pin and one random
> convertor?  How do we know if we got the right ones?  I *think* this
> mostly ends up doing the same thing as the previous version but if
> that's the case why are we doing it?

Hi Mark,

This is the start of conversion to have multiple outputs supported. This
patch only converts to use list so that multiple pins can be used

Actual code to add support for three skl pins is in this series as explained
in the cover letter

> 
> As I've said before one of the reasons it sometimes takes a long time to
> review these things is that there's a lot of big patch serieses getting
> sent which aren't that well explained.
> 
> > -	snd_hdac_codec_write(&edev->hdac, pin_nid, 0,
> > +	snd_hdac_codec_write(&edev->hdac, pin->nid, 0,
> 
> Huge portions of this patch are a simple refactoring like this, it'd be
> good to have split those out as a separate patch so the actual content
> is more visible.

Yes this patch is only refactoring to use list for pins

Thanks
-- 
~Vinod

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

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