PATCH - ESI Juli driver

PATCH - ESI Juli driver

[Pavel Hofman]

[-- Attachment #1: Type: text/plain, Size: 1168 bytes --]

Hi Takashi,

Juli support required some changes to ice1724.c/ice1712.h. Since Juli 
uses different clock scheme than most other ice1712/24 cards, I had to 
move all clock-specific code to card-specific routines. Most of them are 
redefined in juli.c.

The driver is tested, only analog-in and digital-in monitors are not 
working. I have coded according to manufacturer's documentation, the 
same procedure is in OSS Juli driver. I described the issue in detail in 
the code, perhaps someone will do more research.

The card itself has no HW master volume control. It would have to be 
implemented as a virtual control manipulating internal values of PCM and 
monitor volume controls. I will take a look at it later. For now, 
perhaps we could rename the PCM Volume to Master Volume, as most people 
will not need the monitoring anyway. I will leave it to your decision.

Signed-off-by: Pavel Hofman <dustin@seznam.cz>

* moving all clock-specific code to card-specific routines
* support for ESI Juli
* to-be-researched - monitoring of analog/digital inputs
* missing - master volume control, will have to be implemented as 
virtual, no HW support.


Thanks,

Pavel.

[-- Attachment #2: patch-ice-juli.diff.gz --]
[-- Type: application/x-gzip, Size: 10647 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: PATCH - ESI Juli driver

[Takashi Iwai]

At Sun, 16 Mar 2008 13:57:24 +0100,
Pavel Hofman wrote:
> 
> Hi Takashi,
> 
> Juli support required some changes to ice1724.c/ice1712.h. Since Juli 
> uses different clock scheme than most other ice1712/24 cards, I had to 
> move all clock-specific code to card-specific routines. Most of them are 
> redefined in juli.c.

I feel there are too many additional callbacks in your patch.
Since we don't need too subtle tuning, can we simplify this?

Regarding the separate rate setting for each stream, we should
consider about Maya44 support, too.

> The driver is tested, only analog-in and digital-in monitors are not 
> working. I have coded according to manufacturer's documentation, the 
> same procedure is in OSS Juli driver. I described the issue in detail in 
> the code, perhaps someone will do more research.
> 
> The card itself has no HW master volume control. It would have to be 
> implemented as a virtual control manipulating internal values of PCM and 
> monitor volume controls. I will take a look at it later. For now, 
> perhaps we could rename the PCM Volume to Master Volume, as most people 
> will not need the monitoring anyway. I will leave it to your decision.
> 
> Signed-off-by: Pavel Hofman <dustin@seznam.cz>
> 
> * moving all clock-specific code to card-specific routines
> * support for ESI Juli
> * to-be-researched - monitoring of analog/digital inputs
> * missing - master volume control, will have to be implemented as 
> virtual, no HW support.

vmaster can be used for this now.  It was moved to core directory
after 1.0.16 release.


thanks,

Takashi

Re: PATCH - ESI Juli driver

[Pavel Hofman]

Takashi Iwai wrote:
> At Sun, 16 Mar 2008 13:57:24 +0100,
> Pavel Hofman wrote:
>> Hi Takashi,
> 
> I feel there are too many additional callbacks in your patch.
> Since we don't need too subtle tuning, can we simplify this?

There are only two callbacks which Juli does not redefine. These could 
be removed. Though I felt it was cleaner to have all the clock-specific 
stuff potentially redefinable for other cards.

I did not want to copy/paste any algorithmical code from ice1724.c to 
juli.c as it complicates future maintenance. Now, the vt1724 code in 
ice1724.c is pretty much about general algorhitms, specific routines are 
in stdclock_... I tested the original clock with Prodigy192 which 
provided testing analog/SPDIF signals. There are no functional changes.


> 
> Regarding the separate rate setting for each stream, we should
> consider about Maya44 support, too.

I have been following the discussion. Unfortunately, I did not get to 
see any patch of ice1724.c, iirc the last patch included the whole 
modified ice1724.c copied to alsa-driver.

Rainer, what are the changes?

Now each card can specify its own stream parameters. E.g. Juli has a 
modified list of available frequencies for analog streams.


> 
>> The driver is tested, only analog-in and digital-in monitors are not 
>> working. I have coded according to manufacturer's documentation, the 
>> same procedure is in OSS Juli driver. I described the issue in detail in 
>> the code, perhaps someone will do more research.
>>
>> The card itself has no HW master volume control. It would have to be 
>> implemented as a virtual control manipulating internal values of PCM and 
>> monitor volume controls. I will take a look at it later. For now, 
>> perhaps we could rename the PCM Volume to Master Volume, as most people 
>> will not need the monitoring anyway. I will leave it to your decision.
>>
>> Signed-off-by: Pavel Hofman <dustin@seznam.cz>
>>
>> * moving all clock-specific code to card-specific routines
>> * support for ESI Juli
>> * to-be-researched - monitoring of analog/digital inputs
>> * missing - master volume control, will have to be implemented as 
>> virtual, no HW support.
> 
> vmaster can be used for this now.  It was moved to core directory
> after 1.0.16 release.

Thanks, I did not know about vmaster. I will try to implement it tonight.

The Juli support is one of the very called-for features, I guess it 
would be a pitty to keep a functional patch out of repository.

Thanks,

Pavel.

Re: PATCH - ESI Juli driver

[Takashi Iwai]

At Mon, 17 Mar 2008 09:57:49 +0100,
Pavel Hofman wrote:
> 
> Takashi Iwai wrote:
> > At Sun, 16 Mar 2008 13:57:24 +0100,
> > Pavel Hofman wrote:
> >> Hi Takashi,
> > 
> > I feel there are too many additional callbacks in your patch.
> > Since we don't need too subtle tuning, can we simplify this?
> 
> There are only two callbacks which Juli does not redefine. These could 
> be removed. Though I felt it was cleaner to have all the clock-specific 
> stuff potentially redefinable for other cards.
> 
> I did not want to copy/paste any algorithmical code from ice1724.c to 
> juli.c as it complicates future maintenance. Now, the vt1724 code in 
> ice1724.c is pretty much about general algorhitms, specific routines are 
> in stdclock_... I tested the original clock with Prodigy192 which 
> provided testing analog/SPDIF signals. There are no functional changes.

I see the point that you want to make it generic.  But, too many
callbacks, in other words, too many tuning parameters make the code
difficult to follow.  It's a question of balance.


Takashi

Re: PATCH - ESI Juli driver

[Pavel Hofman]

>>
>> I did not want to copy/paste any algorithmical code from ice1724.c to 
>> juli.c as it complicates future maintenance. Now, the vt1724 code in 
>> ice1724.c is pretty much about general algorhitms, specific routines are 
>> in stdclock_... I tested the original clock with Prodigy192 which 
>> provided testing analog/SPDIF signals. There are no functional changes.
> 
> I see the point that you want to make it generic.  But, too many
> callbacks, in other words, too many tuning parameters make the code
> difficult to follow.  It's a question of balance.

Takashi, I understand your concern. On the other hand, if the callbacks 
are well documented, it actually makes the code easier to follow, as it 
separates general algorithms from detailed HW implementation. I can 
certainly add more comments to the callback functions.

Honestly, I do not know which callbacks to remove (apart of those two 
not redefined in Juli). Otherwise I will have to copy/paste serious 
portions of code to juli.c which I would really want to avoid.

As I see it, the clocking code in ice1724 was written for one specific 
implementation. There was no need to do so in a general way and I 
completely understand that. Nevertheless, juli uses a different clock 
implementation and I believe the time has come to make the code in 
ice1724 more general. Unfortunately, I do not know of any other 
technology but callbacks.

What would you recommend?

Thanks,

Pavel.


> 
> 
> Takashi
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

-- 
-----------------

inSITE, s.r.o.

Rubesova 29, 326 00 Plzen
Tel., fax: +420 - 37 - 74 493 58
GSM: +420 - 603 - 163 973
Email: pavel.hofman@insite.cz

www.educity.cz, www.insite.cz
www.meetings.cz, www.hrzive.cz
www.comben.cz, www.hr-online.cz
-------------------------------
Navstivte www.educity.cz, server
s nejvetsi nabidkou profesniho
vzdelavani na ceskem internetu.

Re: PATCH - ESI Juli driver

[Takashi Iwai]

At Mon, 17 Mar 2008 10:37:22 +0100,
Pavel Hofman wrote:
> 
> >>
> >> I did not want to copy/paste any algorithmical code from ice1724.c to 
> >> juli.c as it complicates future maintenance. Now, the vt1724 code in 
> >> ice1724.c is pretty much about general algorhitms, specific routines are 
> >> in stdclock_... I tested the original clock with Prodigy192 which 
> >> provided testing analog/SPDIF signals. There are no functional changes.
> > 
> > I see the point that you want to make it generic.  But, too many
> > callbacks, in other words, too many tuning parameters make the code
> > difficult to follow.  It's a question of balance.
> 
> Takashi, I understand your concern. On the other hand, if the callbacks 
> are well documented, it actually makes the code easier to follow, as it 
> separates general algorithms from detailed HW implementation. I can 
> certainly add more comments to the callback functions.
> 
> Honestly, I do not know which callbacks to remove (apart of those two 
> not redefined in Juli). Otherwise I will have to copy/paste serious 
> portions of code to juli.c which I would really want to avoid.
>
> As I see it, the clocking code in ice1724 was written for one specific 
> implementation. There was no need to do so in a general way and I 
> completely understand that. Nevertheless, juli uses a different clock 
> implementation and I believe the time has come to make the code in 
> ice1724 more general. Unfortunately, I do not know of any other 
> technology but callbacks.
> 
> What would you recommend?

IMO, rate_code can be avoided.  Instead of exposing the encoded value,
better to use the raw rate value as parameters.
And, the texts inf rates_info can be generated dynamically.

So, what we need primarily are callbacks to get and set the current
rate setting.  Suppose rate=0 as SPDIF-in, we can pass the raw rate
value.  Then snd_vt1724_pro_internal_clock_get() would just a function
to get the current rate and compares it with the given rate_info[]
value, returns the index.

How to set stream-specific hw_params is another question.  But surely
we can cut off a bit more.


thanks,

Takashi

Re: PATCH - ESI Juli driver

[Pavel Hofman]

Takashi Iwai wrote:
> At Mon, 17 Mar 2008 10:37:22 +0100,
> Pavel Hofman wrote:

>>
>> What would you recommend?
> 
> IMO, rate_code can be avoided.  Instead of exposing the encoded value,
> better to use the raw rate value as parameters.

No problem. I just wanted to avoid the repeated conversion from the 
numerical rate to the card specific representation which is input 
information for all the rate-related code. ice1724 cards utilize 
MT_RATE, juli makes use of GPIO.

If you are OK with the repeated conversion, I will remove this. I tried 
to keep the methods inline if possible.

> And, the texts inf rates_info can be generated dynamically.

For obvious reasons I tried changing ice1724 as little as possible. That 
is why I kept the original code, only rearranged it.

I can change the way texts in rates_info are generated. Still there will 
have to be some callback as juli has a different list of rates.

> 
> So, what we need primarily are callbacks to get and set the current
> rate setting.  Suppose rate=0 as SPDIF-in, we can pass the raw rate
> value.  Then snd_vt1724_pro_internal_clock_get() would just a function
> to get the current rate and compares it with the given rate_info[]
> value, returns the index.

I am afraid I do not understand what to change in 
snd_vt1724_pro_internal_clock_get(). It seems fairly logical, I made 
only minor changes - is_spdif_master used in other parts of the code, 
get_rate_index with a simple meaning.

> 
> How to set stream-specific hw_params is another question.  But surely
> we can cut off a bit more.

We probably can, by rewriting portions of the original well-tested 
ice1724 code. I really wanted to avoid that and changed by callbacks 
only the card-specific portions.

OK, I will remove the rate_code conversions, the new overhead will be 
low and one abstraction will be removed.

For the rest, please state you objectives. Either cutting a few of the 
callbacks by non-trivial rewrite of the original ice1724 code, or 
keeping the remaining callbacks and the well-tested code.

Thanks a lot,

Pavel.




> 
> 
> thanks,
> 
> Takashi

-- 
-----------------

inSITE, s.r.o.

Rubesova 29, 326 00 Plzen
Tel., fax: +420 - 37 - 74 493 58
GSM: +420 - 603 - 163 973
Email: pavel.hofman@insite.cz

www.educity.cz, www.insite.cz
www.meetings.cz, www.hrzive.cz
www.comben.cz, www.hr-online.cz
-------------------------------
Navstivte www.educity.cz, server
s nejvetsi nabidkou profesniho
vzdelavani na ceskem internetu.

Re: PATCH - ESI Juli driver

[Takashi Iwai]

At Mon, 17 Mar 2008 15:08:48 +0100,
Pavel Hofman wrote:
> 
> Takashi Iwai wrote:
> > At Mon, 17 Mar 2008 10:37:22 +0100,
> > Pavel Hofman wrote:
> 
> >>
> >> What would you recommend?
> > 
> > IMO, rate_code can be avoided.  Instead of exposing the encoded value,
> > better to use the raw rate value as parameters.
> 
> No problem. I just wanted to avoid the repeated conversion from the 
> numerical rate to the card specific representation which is input 
> information for all the rate-related code. ice1724 cards utilize 
> MT_RATE, juli makes use of GPIO.
> 
> If you are OK with the repeated conversion, I will remove this. I tried 
> to keep the methods inline if possible.

Usually you don't have to specify inline expclitily unless it's really
the time-critical code path.  And the code path of the rate setting is
certainly no such a thing.

> > And, the texts inf rates_info can be generated dynamically.
> 
> For obvious reasons I tried changing ice1724 as little as possible. That 
> is why I kept the original code, only rearranged it.
> 
> I can change the way texts in rates_info are generated. Still there will 
> have to be some callback as juli has a different list of rates.
> 
> > So, what we need primarily are callbacks to get and set the current
> > rate setting.  Suppose rate=0 as SPDIF-in, we can pass the raw rate
> > value.  Then snd_vt1724_pro_internal_clock_get() would just a function
> > to get the current rate and compares it with the given rate_info[]
> > value, returns the index.
> 
> I am afraid I do not understand what to change in 
> snd_vt1724_pro_internal_clock_get(). It seems fairly logical, I made 
> only minor changes - is_spdif_master used in other parts of the code, 
> get_rate_index with a simple meaning.

Yeah, your change is logical -- simply convert some code snippets to
callbacks.  But, this isn't really good for maintenance for a long
term.  I prefer a bit more straight way if we really change
something.  And, indeed we do need to change the stuff for rate
settings.

> > How to set stream-specific hw_params is another question.  But surely
> > we can cut off a bit more.
> 
> We probably can, by rewriting portions of the original well-tested 
> ice1724 code. I really wanted to avoid that and changed by callbacks 
> only the card-specific portions.
> 
> OK, I will remove the rate_code conversions, the new overhead will be 
> low and one abstraction will be removed.
> 
> For the rest, please state you objectives. Either cutting a few of the 
> callbacks by non-trivial rewrite of the original ice1724 code, or 
> keeping the remaining callbacks and the well-tested code.

Don't be too nervous about changing the ice1724 code right now :)
We would need the rewrite of core codes anyway because of other
problems in Maya44.  And, I believe we can fix more than we might
break by such a restructuring in the end.  The current ice1724.c is
way too complex due to its history, derived from ice1712.c.


Thanks,

Takashi

Re: PATCH - ESI Juli driver

[Pavel Hofman]

Takashi Iwai wrote:
>> I am afraid I do not understand what to change in 
>> snd_vt1724_pro_internal_clock_get(). It seems fairly logical, I made 
>> only minor changes - is_spdif_master used in other parts of the code, 
>> get_rate_index with a simple meaning.
> 
> Yeah, your change is logical -- simply convert some code snippets to
> callbacks.  But, this isn't really good for maintenance for a long
> term.  I prefer a bit more straight way if we really change
> something.  And, indeed we do need to change the stuff for rate
> settings.

I am afraid I do not have the insight of what needs to be changed. The
code as is seems fairly OK to me, just a bit underdocumented.

> 
>>
>> OK, I will remove the rate_code conversions, the new overhead will be 
>> low and one abstraction will be removed.
>>
>> For the rest, please state you objectives. Either cutting a few of the 
>> callbacks by non-trivial rewrite of the original ice1724 code, or 
>> keeping the remaining callbacks and the well-tested code.
> 
> Don't be too nervous about changing the ice1724 code right now :)
> We would need the rewrite of core codes anyway because of other
> problems in Maya44.  And, I believe we can fix more than we might
> break by such a restructuring in the end.  The current ice1724.c is
> way too complex due to its history, derived from ice1712.c.

Well, to tell the truth, I do not feel knowledgeable enough of the
alsa infrastructure to implement major changes in ice1724 code.

Actually, what is the deal with maya44? Its detailed image shows it has 
standard clocking scheme (no FPGA, PLL etc.), just a couple of Wolfson 
codecs, connection to MI/ODI/O card (fully supported e.g. by 
Prodigy192), switched I2S input between SPDIF-IN/Analog-IN. I just do 
not see a reason to rewrite ice1724 because of maya44 support.

The limitation of capture rate to 96kHz is present in most other ice1724 
cards (including Juli) and nobody has made a big deal of it. It could be 
perhaps tackled in a similar manner we solved the single SPDIF-input rate.

Plus I will have to return the borrowed Juli soon, and will not be able 
to do some qualified testing.

I have another ice1724 card on the way (SndScape Odeum SPDIF interface) 
and really would love to finish Juli soon.


Thanks a lot,

Pavel.

Re: PATCH - ESI Juli driver

[Takashi Iwai]

At Mon, 17 Mar 2008 16:50:39 +0100,
Pavel Hofman wrote:
> 
> Takashi Iwai wrote:
> >> I am afraid I do not understand what to change in 
> >> snd_vt1724_pro_internal_clock_get(). It seems fairly logical, I made 
> >> only minor changes - is_spdif_master used in other parts of the code, 
> >> get_rate_index with a simple meaning.
> > 
> > Yeah, your change is logical -- simply convert some code snippets to
> > callbacks.  But, this isn't really good for maintenance for a long
> > term.  I prefer a bit more straight way if we really change
> > something.  And, indeed we do need to change the stuff for rate
> > settings.
> 
> I am afraid I do not have the insight of what needs to be changed. The
> code as is seems fairly OK to me, just a bit underdocumented.
> 
> > 
> >>
> >> OK, I will remove the rate_code conversions, the new overhead will be 
> >> low and one abstraction will be removed.
> >>
> >> For the rest, please state you objectives. Either cutting a few of the 
> >> callbacks by non-trivial rewrite of the original ice1724 code, or 
> >> keeping the remaining callbacks and the well-tested code.
> > 
> > Don't be too nervous about changing the ice1724 code right now :)
> > We would need the rewrite of core codes anyway because of other
> > problems in Maya44.  And, I believe we can fix more than we might
> > break by such a restructuring in the end.  The current ice1724.c is
> > way too complex due to its history, derived from ice1712.c.
> 
> Well, to tell the truth, I do not feel knowledgeable enough of the
> alsa infrastructure to implement major changes in ice1724 code.
> 
> Actually, what is the deal with maya44? Its detailed image shows it has 
> standard clocking scheme (no FPGA, PLL etc.), just a couple of Wolfson 
> codecs, connection to MI/ODI/O card (fully supported e.g. by 
> Prodigy192), switched I2S input between SPDIF-IN/Analog-IN. I just do 
> not see a reason to rewrite ice1724 because of maya44 support.
> 
> The limitation of capture rate to 96kHz is present in most other ice1724 
> cards (including Juli) and nobody has made a big deal of it. It could be 
> perhaps tackled in a similar manner we solved the single SPDIF-input rate.

Exactly.  That's why I suggested here.

> Plus I will have to return the borrowed Juli soon, and will not be able 
> to do some qualified testing.
> 
> I have another ice1724 card on the way (SndScape Odeum SPDIF interface) 
> and really would love to finish Juli soon.

Well, then we'll need to find some point to compromise.  Could you
repost the patch after reducing the callbacks again?

I'll have also no time from the next week at all -- will be on
vacation for three weeks.  So, let's kick it out soonish :)


thanks,

Takashi

Re: PATCH - ESI Juli driver

[Pavel Hofman]

Takashi Iwai wrote:
> At Mon, 17 Mar 2008 16:50:39 +0100,
> Pavel Hofman wrote:
>> Takashi Iwai wrote:
> 
> Well, then we'll need to find some point to compromise.  Could you
> repost the patch after reducing the callbacks again?
> 
> I'll have also no time from the next week at all -- will be on
> vacation for three weeks.  So, let's kick it out soonish :)
> 
> 
Thanks, I will send the patch tonight.

Pavel.

Re: PATCH - ESI Juli driver

[Pavel Hofman]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

Takashi Iwai wrote:
> At Mon, 17 Mar 2008 16:50:39 +0100,
> Pavel Hofman wrote:
> 
> Well, then we'll need to find some point to compromise.  Could you
> repost the patch after reducing the callbacks again?
> 
> I'll have also no time from the next week at all -- will be on
> vacation for three weeks.  So, let's kick it out soonish :)
> 
Hi,

Here is the patch. I removed several callbacks and changed _rate_code 
callbacks to _rate ones. You were right, the change did clean the code a 
little bit. The vmaster surprisingly works too :)

The compilation writes
In file included from 
/home/pavel/projects/alsa/alsa-driver/pci/ice1712/juli.c:3:
/home/pavel/projects/alsa/alsa-driver/pci/ice1712/../../alsa-kernel/pci/ice1712/juli.c:139: 
warning: function declaration isn’t a prototype

Routine juli_init_gpio_rates() without any parameter starts on that 
line. I do not know how to fix the warning, could you do so, please?

Thanks a lot.

Pavel.


Signed-off-by: Pavel Hofman <dustin at seznam.cz>

* moving most of clock-specific code to card-specific routines
* support for ESI Juli
* to-be-researched - monitoring of analog/digital inputs

[-- Attachment #2: patch-ice-juli-v2.diff.gz --]
[-- Type: application/x-gzip, Size: 11075 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: PATCH - ESI Juli driver - removed debugs

[Pavel Hofman]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

Takashi Iwai wrote:
> At Mon, 17 Mar 2008 16:50:39 +0100,
> Pavel Hofman wrote:
> 
> Well, then we'll need to find some point to compromise.  Could you
> repost the patch after reducing the callbacks again?
> 
> I'll have also no time from the next week at all -- will be on
> vacation for three weeks.  So, let's kick it out soonish :)
> 
Hi,

I am sorry, the previous patch had some debugs enabled. Here is a fixed 
patch.

Thanks a lot.

Pavel.


Signed-off-by: Pavel Hofman <dustin at seznam.cz>

* moving most of clock-specific code to card-specific routines
* support for ESI Juli
* to-be-researched - monitoring of analog/digital inputs


[-- Attachment #2: patch-ice-juli-v3.diff.gz --]
[-- Type: application/x-gzip, Size: 11076 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: PATCH - ESI Juli driver - removed debugs

[Takashi Iwai]

At Tue, 18 Mar 2008 07:56:11 +0100,
Pavel Hofman wrote:
> 
> Takashi Iwai wrote:
> > At Mon, 17 Mar 2008 16:50:39 +0100,
> > Pavel Hofman wrote:
> > 
> > Well, then we'll need to find some point to compromise.  Could you
> > repost the patch after reducing the callbacks again?
> > 
> > I'll have also no time from the next week at all -- will be on
> > vacation for three weeks.  So, let's kick it out soonish :)
> > 
> Hi,
> 
> I am sorry, the previous patch had some debugs enabled. Here is a fixed 
> patch.
> 
> Thanks a lot.
> 
> Pavel.
> 
> 
> Signed-off-by: Pavel Hofman <dustin at seznam.cz>
> 
> * moving most of clock-specific code to card-specific routines
> * support for ESI Juli
> * to-be-researched - monitoring of analog/digital inputs

Thanks.  Now I checked the patch and found it much better now.
However, I still think the get_rate_index can be omitted by checking
the index from the rate list dynamically.  This looks inefficent but
improves the readability and reduces the code size better.  Moreover,
the snd_pcm_hw's aren't needed to be prepared for each type.

Below is my revised patch.  With this patch, ice->hw_rates must be
non-NULL and this is referred in many places.  It cleans up
*_internal_clock*() nicely.


Takashi

---

diff -r 385dab2cd0dd pci/ice1712/ice1712.h
--- a/pci/ice1712/ice1712.h	Tue Mar 18 12:13:03 2008 +0100
+++ b/pci/ice1712/ice1712.h	Tue Mar 18 15:54:50 2008 +0100
@@ -367,6 +367,15 @@ struct snd_ice1712 {
 
 	/* other board-specific data */
 	void *spec;
+
+	/* VT172x specific */
+	int pro_rate_default;
+	int (*is_spdif_master)(struct snd_ice1712 *ice);
+	unsigned int (*get_rate)(struct snd_ice1712 *ice);
+	void (*set_rate)(struct snd_ice1712 *ice, unsigned int rate);
+	unsigned char (*set_mclk)(struct snd_ice1712 *ice, unsigned int rate);
+	void (*set_spdif_clock)(struct snd_ice1712 *ice);
+
 };
 
 
diff -r 385dab2cd0dd pci/ice1712/ice1724.c
--- a/pci/ice1712/ice1724.c	Tue Mar 18 12:13:03 2008 +0100
+++ b/pci/ice1712/ice1724.c	Tue Mar 18 15:54:50 2008 +0100
@@ -106,15 +106,19 @@ static unsigned int PRO_RATE_DEFAULT = 4
  *  Basic I/O
  */
  
+/*
+ *  default rates, default clock routines
+ */
+
 /* check whether the clock mode is spdif-in */
-static inline int is_spdif_master(struct snd_ice1712 *ice)
+static inline int stdclock_is_spdif_master(struct snd_ice1712 *ice)
 {
 	return (inb(ICEMT1724(ice, RATE)) & VT1724_SPDIF_MASTER) ? 1 : 0;
 }
 
 static inline int is_pro_rate_locked(struct snd_ice1712 *ice)
 {
-	return is_spdif_master(ice) || PRO_RATE_LOCKED;
+	return ice->is_spdif_master(ice) || PRO_RATE_LOCKED;
 }
 
 /*
@@ -391,51 +395,61 @@ static int snd_vt1724_pcm_trigger(struct
 #define DMA_PAUSES	(VT1724_RDMA0_PAUSE|VT1724_PDMA0_PAUSE|VT1724_RDMA1_PAUSE|\
 	VT1724_PDMA1_PAUSE|VT1724_PDMA2_PAUSE|VT1724_PDMA3_PAUSE|VT1724_PDMA4_PAUSE)
 
-static int get_max_rate(struct snd_ice1712 *ice)
+static const unsigned int stdclock_rate_list[16] = {
+	48000, 24000, 12000, 9600, 32000, 16000, 8000, 96000, 44100,
+	22050, 11025, 88200, 176400, 0, 192000, 64000
+};
+
+static unsigned int stdclock_get_rate(struct snd_ice1712 *ice)
 {
+	unsigned int rate;
+	rate = stdclock_rate_list[inb(ICEMT1724(ice, RATE)) & 15];
+	return rate;
+}
+
+static void stdclock_set_rate(struct snd_ice1712 *ice, unsigned int rate)
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(stdclock_rate_list); i++) {
+		if (stdclock_rate_list[i] == rate) {
+			outb(i, ICEMT1724(ice, RATE));
+			return;
+		}
+	}
+}
+
+static unsigned char stdclock_set_mclk(struct snd_ice1712 *ice,
+				       unsigned int rate)
+{
+	unsigned char val, old;
+	/* check MT02 */
 	if (ice->eeprom.data[ICE_EEP2_ACLINK] & VT1724_CFG_PRO_I2S) {
-		if ((ice->eeprom.data[ICE_EEP2_I2S] & 0x08) && !ice->vt1720)
-			return 192000;
+		val = old = inb(ICEMT1724(ice, I2S_FORMAT));
+		if (rate > 96000)
+			val |= VT1724_MT_I2S_MCLK_128X; /* 128x MCLK */
 		else
-			return 96000;
-	} else
-		return 48000;
+			val &= ~VT1724_MT_I2S_MCLK_128X; /* 256x MCLK */
+		if (val != old) {
+			outb(val, ICEMT1724(ice, I2S_FORMAT));
+			/* master clock changed */
+			return 1;
+		}
+	}
+	/* no change in master clock */
+	return 0;
 }
 
 static void snd_vt1724_set_pro_rate(struct snd_ice1712 *ice, unsigned int rate,
 				    int force)
 {
 	unsigned long flags;
-	unsigned char val, old;
-	unsigned int i, mclk_change;
+	unsigned char mclk_change;
+	unsigned int i, old_rate;
 
-	if (rate > get_max_rate(ice))
+	if (rate > ice->hw_rates->list[ice->hw_rates->count - 1])
 		return;
-
-	switch (rate) {
-	case 8000: val = 6; break;
-	case 9600: val = 3; break;
-	case 11025: val = 10; break;
-	case 12000: val = 2; break;
-	case 16000: val = 5; break;
-	case 22050: val = 9; break;
-	case 24000: val = 1; break;
-	case 32000: val = 4; break;
-	case 44100: val = 8; break;
-	case 48000: val = 0; break;
-	case 64000: val = 15; break;
-	case 88200: val = 11; break;
-	case 96000: val = 7; break;
-	case 176400: val = 12; break;
-	case 192000: val = 14; break;
-	default:
-		snd_BUG();
-		val = 0;
-		break;
-	}
-
 	spin_lock_irqsave(&ice->reg_lock, flags);
-	if ((inb(ICEMT1724(ice, DMA_CONTROL)) & DMA_STARTS) || 
+	if ((inb(ICEMT1724(ice, DMA_CONTROL)) & DMA_STARTS) ||
 	    (inb(ICEMT1724(ice, DMA_PAUSE)) & DMA_PAUSES)) {
 		/* running? we cannot change the rate now... */
 		spin_unlock_irqrestore(&ice->reg_lock, flags);
@@ -446,9 +460,9 @@ static void snd_vt1724_set_pro_rate(stru
 		return;
 	}
 
-	old = inb(ICEMT1724(ice, RATE));
-	if (force || old != val)
-		outb(val, ICEMT1724(ice, RATE));
+	old_rate = ice->get_rate(ice);
+	if (force || (old_rate != rate))
+		ice->set_rate(ice, rate);
 	else if (rate == ice->cur_rate) {
 		spin_unlock_irqrestore(&ice->reg_lock, flags);
 		return;
@@ -456,19 +470,9 @@ static void snd_vt1724_set_pro_rate(stru
 
 	ice->cur_rate = rate;
 
-	/* check MT02 */
-	mclk_change = 0;
-	if (ice->eeprom.data[ICE_EEP2_ACLINK] & VT1724_CFG_PRO_I2S) {
-		val = old = inb(ICEMT1724(ice, I2S_FORMAT));
-		if (rate > 96000)
-			val |= VT1724_MT_I2S_MCLK_128X; /* 128x MCLK */
-		else
-			val &= ~VT1724_MT_I2S_MCLK_128X; /* 256x MCLK */
-		if (val != old) {
-			outb(val, ICEMT1724(ice, I2S_FORMAT));
-			mclk_change = 1;
-		}
-	}
+	/* setting master clock */
+	mclk_change = ice->set_mclk(ice, rate);
+
 	spin_unlock_irqrestore(&ice->reg_lock, flags);
 
 	if (mclk_change && ice->gpio.i2s_mclk_changed)
@@ -727,43 +731,32 @@ static const struct snd_pcm_hardware snd
 /*
  * set rate constraints
  */
+static void set_std_hw_rates(struct snd_ice1712 *ice)
+{
+	if (ice->eeprom.data[ICE_EEP2_ACLINK] & VT1724_CFG_PRO_I2S) {
+		/* I2S */
+		/* VT1720 doesn't support more than 96kHz */
+		if ((ice->eeprom.data[ICE_EEP2_I2S] & 0x08) && !ice->vt1720)
+			ice->hw_rates = &hw_constraints_rates_192;
+		else
+			ice->hw_rates = &hw_constraints_rates_96;
+	} else {
+		/* ACLINK */
+		ice->hw_rates = &hw_constraints_rates_48;
+	}
+}
+
 static int set_rate_constraints(struct snd_ice1712 *ice,
 				struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	if (ice->hw_rates) {
-		/* hardware specific */
-		runtime->hw.rate_min = ice->hw_rates->list[0];
-		runtime->hw.rate_max = ice->hw_rates->list[ice->hw_rates->count - 1];
-		runtime->hw.rates = SNDRV_PCM_RATE_KNOT;
-		return snd_pcm_hw_constraint_list(runtime, 0,
-						  SNDRV_PCM_HW_PARAM_RATE,
-						  ice->hw_rates);
-	}
-	if (ice->eeprom.data[ICE_EEP2_ACLINK] & VT1724_CFG_PRO_I2S) {
-		/* I2S */
-		/* VT1720 doesn't support more than 96kHz */
-		if ((ice->eeprom.data[ICE_EEP2_I2S] & 0x08) && !ice->vt1720)
-			return snd_pcm_hw_constraint_list(runtime, 0,
-							  SNDRV_PCM_HW_PARAM_RATE,
-							  &hw_constraints_rates_192);
-		else {
-			runtime->hw.rates = SNDRV_PCM_RATE_KNOT |
-				SNDRV_PCM_RATE_8000_96000;
-			runtime->hw.rate_max = 96000;
-			return snd_pcm_hw_constraint_list(runtime, 0,
-							  SNDRV_PCM_HW_PARAM_RATE,
-							  &hw_constraints_rates_96);
-		}
-	} else if (ice->ac97) {
-		/* ACLINK */
-		runtime->hw.rate_max = 48000;
-		runtime->hw.rates = SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_8000_48000;
-		return snd_pcm_hw_constraint_list(runtime, 0,
-						  SNDRV_PCM_HW_PARAM_RATE,
-						  &hw_constraints_rates_48);
-	}
-	return 0;
+
+	runtime->hw.rate_min = ice->hw_rates->list[0];
+	runtime->hw.rate_max = ice->hw_rates->list[ice->hw_rates->count - 1];
+	runtime->hw.rates = SNDRV_PCM_RATE_KNOT;
+	return snd_pcm_hw_constraint_list(runtime, 0,
+					  SNDRV_PCM_HW_PARAM_RATE,
+					  ice->hw_rates);
 }
 
 /* multi-channel playback needs alignment 8x32bit regardless of the channels
@@ -824,7 +817,7 @@ static int snd_vt1724_playback_pro_close
 	struct snd_ice1712 *ice = snd_pcm_substream_chip(substream);
 
 	if (PRO_RATE_RESET)
-		snd_vt1724_set_pro_rate(ice, PRO_RATE_DEFAULT, 0);
+		snd_vt1724_set_pro_rate(ice, ice->pro_rate_default, 0);
 	ice->playback_pro_substream = NULL;
 
 	return 0;
@@ -835,7 +828,7 @@ static int snd_vt1724_capture_pro_close(
 	struct snd_ice1712 *ice = snd_pcm_substream_chip(substream);
 
 	if (PRO_RATE_RESET)
-		snd_vt1724_set_pro_rate(ice, PRO_RATE_DEFAULT, 0);
+		snd_vt1724_set_pro_rate(ice, ice->pro_rate_default, 0);
 	ice->capture_pro_substream = NULL;
 	return 0;
 }
@@ -980,7 +973,7 @@ static int snd_vt1724_playback_spdif_clo
 	struct snd_ice1712 *ice = snd_pcm_substream_chip(substream);
 
 	if (PRO_RATE_RESET)
-		snd_vt1724_set_pro_rate(ice, PRO_RATE_DEFAULT, 0);
+		snd_vt1724_set_pro_rate(ice, ice->pro_rate_default, 0);
 	ice->playback_con_substream = NULL;
 	if (ice->spdif.ops.close)
 		ice->spdif.ops.close(ice, substream);
@@ -1016,7 +1009,7 @@ static int snd_vt1724_capture_spdif_clos
 	struct snd_ice1712 *ice = snd_pcm_substream_chip(substream);
 
 	if (PRO_RATE_RESET)
-		snd_vt1724_set_pro_rate(ice, PRO_RATE_DEFAULT, 0);
+		snd_vt1724_set_pro_rate(ice, ice->pro_rate_default, 0);
 	ice->capture_con_substream = NULL;
 	if (ice->spdif.ops.close)
 		ice->spdif.ops.close(ice, substream);
@@ -1162,7 +1155,7 @@ static int snd_vt1724_playback_indep_clo
 	struct snd_ice1712 *ice = snd_pcm_substream_chip(substream);
 
 	if (PRO_RATE_RESET)
-		snd_vt1724_set_pro_rate(ice, PRO_RATE_DEFAULT, 0);
+		snd_vt1724_set_pro_rate(ice, ice->pro_rate_default, 0);
 	ice->playback_con_substream_ds[substream->number] = NULL;
 	ice->pcm_reserved[substream->number] = NULL;
 
@@ -1580,50 +1573,18 @@ static int snd_vt1724_pro_internal_clock
 static int snd_vt1724_pro_internal_clock_info(struct snd_kcontrol *kcontrol,
 					      struct snd_ctl_elem_info *uinfo)
 {
-	static const char * const texts_1724[] = {
-		"8000",		/* 0: 6 */
-		"9600",		/* 1: 3 */
-		"11025",	/* 2: 10 */
-		"12000",	/* 3: 2 */
-		"16000",	/* 4: 5 */
-		"22050",	/* 5: 9 */
-		"24000",	/* 6: 1 */
-		"32000",	/* 7: 4 */
-		"44100",	/* 8: 8 */
-		"48000",	/* 9: 0 */
-		"64000",	/* 10: 15 */
-		"88200",	/* 11: 11 */
-		"96000",	/* 12: 7 */
-		"176400",	/* 13: 12 */
-		"192000",	/* 14: 14 */
-		"IEC958 Input",	/* 15: -- */
-	};
-	static const char * const texts_1720[] = {
-		"8000",		/* 0: 6 */
-		"9600",		/* 1: 3 */
-		"11025",	/* 2: 10 */
-		"12000",	/* 3: 2 */
-		"16000",	/* 4: 5 */
-		"22050",	/* 5: 9 */
-		"24000",	/* 6: 1 */
-		"32000",	/* 7: 4 */
-		"44100",	/* 8: 8 */
-		"48000",	/* 9: 0 */
-		"64000",	/* 10: 15 */
-		"88200",	/* 11: 11 */
-		"96000",	/* 12: 7 */
-		"IEC958 Input",	/* 13: -- */
-	};
 	struct snd_ice1712 *ice = snd_kcontrol_chip(kcontrol);
 
 	uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
 	uinfo->count = 1;
-	uinfo->value.enumerated.items = ice->vt1720 ? 14 : 16;
+	uinfo->value.enumerated.items = ice->hw_rates->count + 1;
 	if (uinfo->value.enumerated.item >= uinfo->value.enumerated.items)
 		uinfo->value.enumerated.item = uinfo->value.enumerated.items - 1;
-	strcpy(uinfo->value.enumerated.name,
-	       ice->vt1720 ? texts_1720[uinfo->value.enumerated.item] :
-	       texts_1724[uinfo->value.enumerated.item]);
+	if (uinfo->value.enumerated.item == uinfo->value.enumerated.items - 1)
+		strcpy(uinfo->value.enumerated.name, "IEC958 Input");
+	else
+		sprintf(uinfo->value.enumerated.name, "%d",
+			ice->hw_rates->list[uinfo->value.enumerated.item]);
 	return 0;
 }
 
@@ -1631,68 +1592,79 @@ static int snd_vt1724_pro_internal_clock
 					     struct snd_ctl_elem_value *ucontrol)
 {
 	struct snd_ice1712 *ice = snd_kcontrol_chip(kcontrol);
-	static const unsigned char xlate[16] = {
-		9, 6, 3, 1, 7, 4, 0, 12, 8, 5, 2, 11, 13, 255, 14, 10
-	};
-	unsigned char val;
+	unsigned int i, rate;
 	
 	spin_lock_irq(&ice->reg_lock);
-	if (is_spdif_master(ice)) {
-		ucontrol->value.enumerated.item[0] = ice->vt1720 ? 13 : 15;
+	if (ice->is_spdif_master(ice)) {
+		ucontrol->value.enumerated.item[0] = ice->hw_rates->count;
 	} else {
-		val = xlate[inb(ICEMT1724(ice, RATE)) & 15];
-		if (val == 255) {
-			snd_BUG();
-			val = 0;
+		rate = ice->get_rate(ice);
+		ucontrol->value.enumerated.item[0] = 0;
+		for (i = 0; i < ice->hw_rates->count; i++) {
+			if (ice->hw_rates->list[i] == rate) {
+				ucontrol->value.enumerated.item[0] = i;
+				break;
+			}
 		}
-		ucontrol->value.enumerated.item[0] = val;
 	}
 	spin_unlock_irq(&ice->reg_lock);
 	return 0;
+}
+
+/* setting clock to external - SPDIF */
+static void stdclock_set_spdif_clock(struct snd_ice1712 *ice)
+{
+	unsigned char oval;
+	unsigned char i2s_oval;
+	oval = inb(ICEMT1724(ice, RATE));
+	outb(oval | VT1724_SPDIF_MASTER, ICEMT1724(ice, RATE));
+	/* setting 256fs */
+	i2s_oval = inb(ICEMT1724(ice, I2S_FORMAT));
+	outb(i2s_oval & ~VT1724_MT_I2S_MCLK_128X, ICEMT1724(ice, I2S_FORMAT));
 }
 
 static int snd_vt1724_pro_internal_clock_put(struct snd_kcontrol *kcontrol,
 					     struct snd_ctl_elem_value *ucontrol)
 {
 	struct snd_ice1712 *ice = snd_kcontrol_chip(kcontrol);
-	unsigned char oval;
-	int rate;
-	int change = 0;
-	int spdif = ice->vt1720 ? 13 : 15;
+	unsigned int old_rate, new_rate;
+	unsigned int item = ucontrol->value.enumerated.item[0];
+	unsigned int spdif = ice->hw_rates->count;
+
+	if (item > spdif)
+		return -EINVAL;
 
 	spin_lock_irq(&ice->reg_lock);
-	oval = inb(ICEMT1724(ice, RATE));
-	if (ucontrol->value.enumerated.item[0] == spdif) {
-		unsigned char i2s_oval;
-		outb(oval | VT1724_SPDIF_MASTER, ICEMT1724(ice, RATE));
-		/* setting 256fs */
-		i2s_oval = inb(ICEMT1724(ice, I2S_FORMAT));
-		outb(i2s_oval & ~VT1724_MT_I2S_MCLK_128X,
-		     ICEMT1724(ice, I2S_FORMAT));
+	if (ice->is_spdif_master(ice))
+		old_rate = 0;
+	else
+		old_rate = ice->get_rate(ice);
+	if (item == spdif) {
+		/* switching to external clock via SPDIF */
+		ice->set_spdif_clock(ice);
+		new_rate = 0;
 	} else {
-		rate = rates[ucontrol->value.integer.value[0] % 15];
-		if (rate <= get_max_rate(ice)) {
-			PRO_RATE_DEFAULT = rate;
-			spin_unlock_irq(&ice->reg_lock);
-			snd_vt1724_set_pro_rate(ice, PRO_RATE_DEFAULT, 1);
-			spin_lock_irq(&ice->reg_lock);
+		/* internal on-card clock */
+		new_rate = ice->hw_rates->list[item];
+		ice->pro_rate_default = new_rate;
+		spin_unlock_irq(&ice->reg_lock);
+		snd_vt1724_set_pro_rate(ice, ice->pro_rate_default, 1);
+		spin_lock_irq(&ice->reg_lock);
+	}
+	spin_unlock_irq(&ice->reg_lock);
+
+	/* the first reset to the SPDIF master mode? */
+	if (old_rate != new_rate && !new_rate) {
+		/* notify akm chips as well */
+		unsigned int i;
+		if (ice->gpio.set_pro_rate)
+			ice->gpio.set_pro_rate(ice, 0);
+		for (i = 0; i < ice->akm_codecs; i++) {
+			if (ice->akm[i].ops.set_rate_val)
+				ice->akm[i].ops.set_rate_val(&ice->akm[i], 0);
 		}
 	}
-	change = inb(ICEMT1724(ice, RATE)) != oval;
-	spin_unlock_irq(&ice->reg_lock);
-
-	if ((oval & VT1724_SPDIF_MASTER) !=
-	    (inb(ICEMT1724(ice, RATE)) & VT1724_SPDIF_MASTER)) {
-		/* notify akm chips as well */
-		if (is_spdif_master(ice)) {
-			unsigned int i;
-			for (i = 0; i < ice->akm_codecs; i++) {
-				if (ice->akm[i].ops.set_rate_val)
-					ice->akm[i].ops.set_rate_val(&ice->akm[i], 0);
-			}
-		}
-	}
-	return change;
+	return old_rate != new_rate;
 }
 
 static struct snd_kcontrol_new snd_vt1724_pro_internal_clock __devinitdata = {
@@ -2343,6 +2315,19 @@ static int __devinit snd_vt1724_probe(st
         * was called so in ice1712 driver, and vt1724 driver is derived from
         * ice1712 driver.
         */
+	ice->pro_rate_default = PRO_RATE_DEFAULT;
+	if (!ice->is_spdif_master)
+		ice->is_spdif_master = stdclock_is_spdif_master;
+	if (!ice->get_rate)
+		ice->get_rate = stdclock_get_rate;
+	if (!ice->set_rate)
+		ice->set_rate = stdclock_set_rate;
+	if (!ice->set_mclk)
+		ice->set_mclk = stdclock_set_mclk;
+	if (!ice->set_spdif_clock)
+		ice->set_spdif_clock = stdclock_set_spdif_clock;
+	if (!ice->hw_rates)
+		set_std_hw_rates(ice);
 
 	if ((err = snd_vt1724_pcm_profi(ice, pcm_dev++)) < 0) {
 		snd_card_free(card);
diff -r 385dab2cd0dd pci/ice1712/juli.c
--- a/pci/ice1712/juli.c	Tue Mar 18 12:13:03 2008 +0100
+++ b/pci/ice1712/juli.c	Tue Mar 18 15:54:50 2008 +0100
@@ -4,6 +4,8 @@
  *   Lowlevel functions for ESI Juli@ cards
  *
  *	Copyright (c) 2004 Jaroslav Kysela <perex@perex.cz>
+ *	              2008 Pavel Hofman <dustin@seznam.cz>
+ *
  *
  *   This program is free software; you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -27,11 +29,11 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <sound/core.h>
+#include <sound/tlv.h>
 
 #include "ice1712.h"
 #include "envy24ht.h"
 #include "juli.h"
-
 struct juli_spec {
 	struct ak4114 *ak4114;
 	unsigned int analog: 1;
@@ -42,6 +44,32 @@ struct juli_spec {
  */
 #define AK4114_ADDR		0x20		/* S/PDIF receiver */
 #define AK4358_ADDR		0x22		/* DAC */
+
+/*
+ * Juli does not use the standard ICE1724 clock scheme. Juli's ice1724 chip is
+ * supplied by external clock provided by Xilinx array and MK73-1 PLL frequency
+ * multiplier. Actual frequency is set by ice1724 GPIOs hooked to the Xilinx.
+ *
+ * The clock circuitry is supplied by the two ice1724 crystals. This
+ * arrangement allows to generate independent clock signal for AK4114's input
+ * rate detection circuit. As a result, Juli, unlike most other
+ * ice1724+ak4114-based cards, detects spdif input rate correctly.
+ * This fact is applied in the driver, allowing to modify PCM stream rate
+ * parameter according to the actual input rate.
+ *
+ * Juli uses the remaining three stereo-channels of its DAC to optionally
+ * monitor analog input, digital input, and digital output. The corresponding
+ * I2S signals are routed by Xilinx, controlled by GPIOs.
+ *
+ * The master mute is implemented using output muting transistors (GPIO) in
+ * combination with smuting the DAC.
+ *
+ * The card itself has no HW master volume control, implemented using the
+ * vmaster control.
+ *
+ * TODO:
+ * researching and fixing the input monitors
+ */
 
 /*
  * GPIO pins
@@ -55,17 +83,82 @@ struct juli_spec {
 #define GPIO_MULTI_2X		(1<<2)
 #define GPIO_MULTI_1X		(2<<2)		/* also external */
 #define GPIO_MULTI_HALF		(3<<2)
-#define GPIO_INTERNAL_CLOCK	(1<<4)
+#define GPIO_INTERNAL_CLOCK	(1<<4)		/* 0 = external, 1 = internal */
+#define GPIO_CLOCK_MASK		(1<<4)
 #define GPIO_ANALOG_PRESENT	(1<<5)		/* RO only: 0 = present */
 #define GPIO_RXMCLK_SEL		(1<<7)		/* must be 0 */
 #define GPIO_AK5385A_CKS0	(1<<8)
-#define GPIO_AK5385A_DFS0	(1<<9)		/* swapped with DFS1 according doc? */
-#define GPIO_AK5385A_DFS1	(1<<10)
+#define GPIO_AK5385A_DFS1	(1<<9)
+#define GPIO_AK5385A_DFS0	(1<<10)
 #define GPIO_DIGOUT_MONITOR	(1<<11)		/* 1 = active */
 #define GPIO_DIGIN_MONITOR	(1<<12)		/* 1 = active */
 #define GPIO_ANAIN_MONITOR	(1<<13)		/* 1 = active */
-#define GPIO_AK5385A_MCLK	(1<<14)		/* must be 0 */
-#define GPIO_MUTE_CONTROL	(1<<15)		/* 0 = off, 1 = on */
+#define GPIO_AK5385A_CKS1	(1<<14)		/* must be 0 */
+#define GPIO_MUTE_CONTROL	(1<<15)		/* output mute, 1 = muted */
+
+#define GPIO_RATE_MASK		(GPIO_FREQ_MASK | GPIO_MULTI_MASK | \
+		GPIO_CLOCK_MASK)
+#define GPIO_AK5385A_MASK	(GPIO_AK5385A_CKS0 | GPIO_AK5385A_DFS0 | \
+		GPIO_AK5385A_DFS1 | GPIO_AK5385A_CKS1)
+
+#define JULI_PCM_RATE	(SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_22050 | \
+		SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | \
+		SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_64000 | \
+		SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 | \
+		SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000)
+
+#define GPIO_RATE_16000		(GPIO_FREQ_32KHZ | GPIO_MULTI_HALF | \
+		GPIO_INTERNAL_CLOCK)
+#define GPIO_RATE_22050		(GPIO_FREQ_44KHZ | GPIO_MULTI_HALF | \
+		GPIO_INTERNAL_CLOCK)
+#define GPIO_RATE_24000		(GPIO_FREQ_48KHZ | GPIO_MULTI_HALF | \
+		GPIO_INTERNAL_CLOCK)
+#define GPIO_RATE_32000		(GPIO_FREQ_32KHZ | GPIO_MULTI_1X | \
+		GPIO_INTERNAL_CLOCK)
+#define GPIO_RATE_44100		(GPIO_FREQ_44KHZ | GPIO_MULTI_1X | \
+		GPIO_INTERNAL_CLOCK)
+#define GPIO_RATE_48000		(GPIO_FREQ_48KHZ | GPIO_MULTI_1X | \
+		GPIO_INTERNAL_CLOCK)
+#define GPIO_RATE_64000		(GPIO_FREQ_32KHZ | GPIO_MULTI_2X | \
+		GPIO_INTERNAL_CLOCK)
+#define GPIO_RATE_88200		(GPIO_FREQ_44KHZ | GPIO_MULTI_2X | \
+		GPIO_INTERNAL_CLOCK)
+#define GPIO_RATE_96000		(GPIO_FREQ_48KHZ | GPIO_MULTI_2X | \
+		GPIO_INTERNAL_CLOCK)
+#define GPIO_RATE_176400	(GPIO_FREQ_44KHZ | GPIO_MULTI_4X | \
+		GPIO_INTERNAL_CLOCK)
+#define GPIO_RATE_192000	(GPIO_FREQ_48KHZ | GPIO_MULTI_4X | \
+		GPIO_INTERNAL_CLOCK)
+
+/*
+ * Initial setup of the conversion array GPIO <-> rate
+ */
+static unsigned int juli_rates[] = {
+	16000, 22050, 24000, 32000,
+	44100, 48000, 64000, 88200,
+	96000, 176400, 192000,
+};
+
+static unsigned int gpio_vals[] = {
+	GPIO_RATE_16000, GPIO_RATE_22050, GPIO_RATE_24000, GPIO_RATE_32000,
+	GPIO_RATE_44100, GPIO_RATE_48000, GPIO_RATE_64000, GPIO_RATE_88200,
+	GPIO_RATE_96000, GPIO_RATE_176400, GPIO_RATE_192000,
+};
+
+static struct snd_pcm_hw_constraint_list juli_rates_info = {
+	.count = ARRAY_SIZE(juli_rates),
+	.list = juli_rates,
+	.mask = 0,
+};
+
+static int get_gpio_val(int rate)
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(juli_rates); i++)
+		if (juli_rates[i] == rate)
+			return gpio_vals[i];
+	return 0;
+}
 
 static void juli_ak4114_write(void *private_data, unsigned char reg, unsigned char val)
 {
@@ -77,6 +170,10 @@ static unsigned char juli_ak4114_read(vo
 	return snd_vt1724_read_i2c((struct snd_ice1712 *)private_data, AK4114_ADDR, reg);
 }
 
+/*
+ * If SPDIF capture and slaved to SPDIF-IN, setting runtime rate
+ * to the external rate
+ */
 static void juli_spdif_in_open(struct snd_ice1712 *ice,
 			       struct snd_pcm_substream *substream)
 {
@@ -84,7 +181,8 @@ static void juli_spdif_in_open(struct sn
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	int rate;
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK ||
+			!ice->is_spdif_master(ice))
 		return;
 	rate = snd_ak4114_external_rate(spec->ak4114);
 	if (rate >= runtime->hw.rate_min && rate <= runtime->hw.rate_max) {
@@ -115,57 +213,285 @@ static void juli_akm_write(struct snd_ak
 }
 
 /*
- * change the rate of envy24HT, AK4358
+ * change the rate of envy24HT, AK4358, AK5385
  */
 static void juli_akm_set_rate_val(struct snd_akm4xxx *ak, unsigned int rate)
 {
-	unsigned char old, tmp, dfs;
+	unsigned char old, tmp, ak4358_dfs;
+	unsigned int ak5385_pins, old_gpio, new_gpio;
+	struct snd_ice1712 *ice = ak->private_data[0];
+	struct juli_spec *spec = ice->spec;
 
-	if (rate == 0)  /* no hint - S/PDIF input is master, simply return */
+	if (rate == 0)  /* no hint - S/PDIF input is master or the new spdif
+			   input rate undetected, simply return */
 		return;
-	
+
 	/* adjust DFS on codecs */
-	if (rate > 96000) 
-		dfs = 2;
-	else if (rate > 48000)
-		dfs = 1;
-	else
-		dfs = 0;
-	
+	if (rate > 96000)  {
+		ak4358_dfs = 2;
+		ak5385_pins = GPIO_AK5385A_DFS1 | GPIO_AK5385A_CKS0;
+	} else if (rate > 48000) {
+		ak4358_dfs = 1;
+		ak5385_pins = GPIO_AK5385A_DFS0;
+	} else {
+		ak4358_dfs = 0;
+		ak5385_pins = 0;
+	}
+	/* AK5385 first, since it requires cold reset affecting both codecs */
+	old_gpio = ice->gpio.get_data(ice);
+	new_gpio =  (old_gpio & ~GPIO_AK5385A_MASK) | ak5385_pins;
+	/* printk(KERN_DEBUG "JULI - ak5385 set_rate_val: new gpio 0x%x\n",
+		new_gpio); */
+	ice->gpio.set_data(ice, new_gpio);
+
+	/* cold reset */
+	old = inb(ICEMT1724(ice, AC97_CMD));
+	outb(old | VT1724_AC97_COLD, ICEMT1724(ice, AC97_CMD));
+	udelay(1);
+	outb(old & ~VT1724_AC97_COLD, ICEMT1724(ice, AC97_CMD));
+
+	/* AK4358 */
+	/* set new value, reset DFS */
 	tmp = snd_akm4xxx_get(ak, 0, 2);
-	old = (tmp >> 4) & 0x03;
-	if (old == dfs)
-		return;
-	/* reset DFS */
 	snd_akm4xxx_reset(ak, 1);
 	tmp = snd_akm4xxx_get(ak, 0, 2);
 	tmp &= ~(0x03 << 4);
-	tmp |= dfs << 4;
+	tmp |= ak4358_dfs << 4;
 	snd_akm4xxx_set(ak, 0, 2, tmp);
 	snd_akm4xxx_reset(ak, 0);
+
+	/* reinit ak4114 */
+	snd_ak4114_reinit(spec->ak4114);
 }
+
+#define AK_DAC(xname, xch)	{ .name = xname, .num_channels = xch }
+#define PCM_VOLUME		"PCM Playback Volume"
+#define MONITOR_AN_IN_VOLUME	"Monitor Analog In Volume"
+#define MONITOR_DIG_IN_VOLUME	"Monitor Digital In Volume"
+#define MONITOR_DIG_OUT_VOLUME	"Monitor Digital Out Volume"
+
+static const struct snd_akm4xxx_dac_channel juli_dac[] = {
+	AK_DAC(PCM_VOLUME, 2),
+	AK_DAC(MONITOR_AN_IN_VOLUME, 2),
+	AK_DAC(MONITOR_DIG_OUT_VOLUME, 2),
+	AK_DAC(MONITOR_DIG_IN_VOLUME, 2),
+};
+
 
 static struct snd_akm4xxx akm_juli_dac __devinitdata = {
 	.type = SND_AK4358,
-	.num_dacs = 2,
+	.num_dacs = 8,	/* DAC1 - analog out
+			   DAC2 - analog in monitor
+			   DAC3 - digital out monitor
+			   DAC4 - digital in monitor
+			 */
 	.ops = {
 		.lock = juli_akm_lock,
 		.unlock = juli_akm_unlock,
 		.write = juli_akm_write,
 		.set_rate_val = juli_akm_set_rate_val
+	},
+	.dac_info = juli_dac,
+};
+
+#define juli_mute_info		snd_ctl_boolean_mono_info
+
+static int juli_mute_get(struct snd_kcontrol *kcontrol,
+		struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_ice1712 *ice = snd_kcontrol_chip(kcontrol);
+	unsigned int val;
+	val = ice->gpio.get_data(ice) & (unsigned int) kcontrol->private_value;
+	if (kcontrol->private_value == GPIO_MUTE_CONTROL)
+		/* val 0 = signal on */
+		ucontrol->value.integer.value[0] = (val) ? 0 : 1;
+	else
+		/* val 1 = signal on */
+		ucontrol->value.integer.value[0] = (val) ? 1 : 0;
+	return 0;
+}
+
+static int juli_mute_put(struct snd_kcontrol *kcontrol,
+		struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_ice1712 *ice = snd_kcontrol_chip(kcontrol);
+	unsigned int old_gpio, new_gpio;
+	old_gpio = ice->gpio.get_data(ice);
+	if (ucontrol->value.integer.value[0]) {
+		/* unmute */
+		if (kcontrol->private_value == GPIO_MUTE_CONTROL) {
+			/* 0 = signal on */
+			new_gpio = old_gpio & ~GPIO_MUTE_CONTROL;
+			/* un-smuting DAC */
+			snd_akm4xxx_write(ice->akm, 0, 0x01, 0x01);
+		} else
+			/* 1 = signal on */
+			new_gpio =  old_gpio |
+				(unsigned int) kcontrol->private_value;
+	} else {
+		/* mute */
+		if (kcontrol->private_value == GPIO_MUTE_CONTROL) {
+			/* 1 = signal off */
+			new_gpio = old_gpio | GPIO_MUTE_CONTROL;
+			/* smuting DAC */
+			snd_akm4xxx_write(ice->akm, 0, 0x01, 0x03);
+		} else
+			/* 0 = signal off */
+			new_gpio =  old_gpio &
+				~((unsigned int) kcontrol->private_value);
 	}
+	/* printk("JULI - mute/unmute: control_value: 0x%x, old_gpio: 0x%x, \
+		new_gpio 0x%x\n",
+		(unsigned int)ucontrol->value.integer.value[0], old_gpio,
+		new_gpio); */
+	if (old_gpio != new_gpio) {
+		ice->gpio.set_data(ice, new_gpio);
+		return 1;
+	}
+	/* no change */
+	return 0;
+}
+
+static struct snd_kcontrol_new juli_mute_controls[] __devinitdata = {
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "Master Playback Switch",
+		.info = juli_mute_info,
+		.get = juli_mute_get,
+		.put = juli_mute_put,
+		.private_value = GPIO_MUTE_CONTROL,
+	},
+	/* Although the following functionality respects the succint NDA'd
+	 * documentation from the card manufacturer, and the same way of
+	 * operation is coded in OSS Juli driver, only Digital Out monitor
+	 * seems to work. Surprisingly, Analog input monitor outputs Digital
+	 * output data. The two are independent, as enabling both doubles
+	 * volume of the monitor sound.
+	 *
+	 * Checking traces on the board suggests the functionality described
+	 * by the manufacturer is correct - I2S from ADC and AK4114
+	 * go to ICE as well as to Xilinx, I2S inputs of DAC2,3,4 (the monitor
+	 * inputs) are fed from Xilinx.
+	 *
+	 * I even checked traces on board and coded a support in driver for
+	 * an alternative possiblity - the unused I2S ICE output channels
+	 * switched to HW-IN/SPDIF-IN and providing the monitoring signal to
+	 * the DAC - to no avail. The I2S outputs seem to be unconnected.
+	 *
+	 * The windows driver supports the monitoring correctly.
+	 */
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "Monitor Analog In Switch",
+		.info = juli_mute_info,
+		.get = juli_mute_get,
+		.put = juli_mute_put,
+		.private_value = GPIO_ANAIN_MONITOR,
+	},
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "Monitor Digital Out Switch",
+		.info = juli_mute_info,
+		.get = juli_mute_get,
+		.put = juli_mute_put,
+		.private_value = GPIO_DIGOUT_MONITOR,
+	},
+	{
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = "Monitor Digital In Switch",
+		.info = juli_mute_info,
+		.get = juli_mute_get,
+		.put = juli_mute_put,
+		.private_value = GPIO_DIGIN_MONITOR,
+	},
 };
+
+
+static void ak4358_proc_regs_read(struct snd_info_entry *entry,
+		struct snd_info_buffer *buffer)
+{
+	struct snd_ice1712 *ice = (struct snd_ice1712 *)entry->private_data;
+	int reg, val;
+	for (reg = 0; reg <= 0xf; reg++) {
+		val =  snd_akm4xxx_get(ice->akm, 0, reg);
+		snd_iprintf(buffer, "0x%02x = 0x%02x\n", reg, val);
+	}
+}
+
+static void ak4358_proc_init(struct snd_ice1712 *ice)
+{
+	struct snd_info_entry *entry;
+	if (!snd_card_proc_new(ice->card, "ak4358_codec", &entry))
+		snd_info_set_text_ops(entry, ice, ak4358_proc_regs_read);
+}
+
+static char *slave_vols[] __devinitdata = {
+	PCM_VOLUME,
+	MONITOR_AN_IN_VOLUME,
+	MONITOR_DIG_IN_VOLUME,
+	MONITOR_DIG_OUT_VOLUME,
+	NULL
+};
+
+static __devinitdata
+DECLARE_TLV_DB_SCALE(juli_master_db_scale, -6350, 50, 1);
+
+static struct snd_kcontrol __devinit *ctl_find(struct snd_card *card,
+		const char *name)
+{
+	struct snd_ctl_elem_id sid;
+	memset(&sid, 0, sizeof(sid));
+	/* FIXME: strcpy is bad. */
+	strcpy(sid.name, name);
+	sid.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	return snd_ctl_find_id(card, &sid);
+}
+
+static void __devinit add_slaves(struct snd_card *card,
+				 struct snd_kcontrol *master, char **list)
+{
+	for (; *list; list++) {
+		struct snd_kcontrol *slave = ctl_find(card, *list);
+		/* printk(KERN_DEBUG "add_slaves - %s\n", *list); */
+		if (slave) {
+			/* printk(KERN_DEBUG "slave %s found\n", *list); */
+			snd_ctl_add_slave(master, slave);
+		}
+	}
+}
 
 static int __devinit juli_add_controls(struct snd_ice1712 *ice)
 {
 	struct juli_spec *spec = ice->spec;
 	int err;
+	unsigned int i;
+	struct snd_kcontrol *vmaster;
+
 	err = snd_ice1712_akm4xxx_build_controls(ice);
 	if (err < 0)
 		return err;
+
+	for (i = 0; i < ARRAY_SIZE(juli_mute_controls); i++) {
+		err = snd_ctl_add(ice->card,
+				snd_ctl_new1(&juli_mute_controls[i], ice));
+		if (err < 0)
+			return err;
+	}
+	/* Create virtual master control */
+	vmaster = snd_ctl_make_virtual_master("Master Playback Volume",
+					      juli_master_db_scale);
+	if (!vmaster)
+		return -ENOMEM;
+	add_slaves(ice->card, vmaster, slave_vols);
+	err = snd_ctl_add(ice->card, vmaster);
+	if (err < 0)
+		return err;
+
 	/* only capture SPDIF over AK4114 */
 	err = snd_ak4114_build(spec->ak4114, NULL,
-			       ice->pcm_pro->streams[SNDRV_PCM_STREAM_CAPTURE].substream);
+			ice->pcm->streams[SNDRV_PCM_STREAM_CAPTURE].substream);
+
+	ak4358_proc_init(ice);
 	if (err < 0)
 		return err;
 	return 0;
@@ -174,6 +500,74 @@ static int __devinit juli_add_controls(s
 /*
  * initialize the chip
  */
+
+static inline int juli_is_spdif_master(struct snd_ice1712 *ice)
+{
+	return (ice->gpio.get_data(ice) & GPIO_INTERNAL_CLOCK) ? 0 : 1;
+}
+
+static unsigned int juli_get_rate(struct snd_ice1712 *ice)
+{
+	int i;
+	unsigned char result;
+
+	result =  ice->gpio.get_data(ice) & GPIO_RATE_MASK;
+	for (i = 0; i < ARRAY_SIZE(gpio_vals); i++)
+		if (gpio_vals[i] == result)
+			return juli_rates[i];
+	return 0;
+}
+
+/* setting new rate */
+static void juli_set_rate(struct snd_ice1712 *ice, unsigned int rate)
+{
+	unsigned int old, new;
+	unsigned char val;
+
+	old = ice->gpio.get_data(ice);
+	new =  (old & ~GPIO_RATE_MASK) | get_gpio_val(rate);
+	/* printk(KERN_DEBUG "JULI - set_rate: old %x, new %x\n",
+			old & GPIO_RATE_MASK,
+			new & GPIO_RATE_MASK); */
+
+	ice->gpio.set_data(ice, new);
+	/* switching to external clock - supplied by external circuits */
+	val = inb(ICEMT1724(ice, RATE));
+	outb(val | VT1724_SPDIF_MASTER, ICEMT1724(ice, RATE));
+}
+
+static inline unsigned char juli_set_mclk(struct snd_ice1712 *ice,
+					  unsigned int rate)
+{
+	/* no change in master clock */
+	return 0;
+}
+
+/* setting clock to external - SPDIF */
+static void juli_set_spdif_clock(struct snd_ice1712 *ice)
+{
+	unsigned int old;
+	old = ice->gpio.get_data(ice);
+	/* external clock (= 0), multiply 1x, 48kHz */
+	ice->gpio.set_data(ice, (old & ~GPIO_RATE_MASK) | GPIO_MULTI_1X |
+			GPIO_FREQ_48KHZ);
+}
+
+/* Called when ak4114 detects change in the input SPDIF stream */
+static void juli_ak4114_change(struct ak4114 *ak4114, unsigned char c0,
+			       unsigned char c1)
+{
+	struct snd_ice1712 *ice = ak4114->change_callback_private;
+	int rate;
+	if (ice->is_spdif_master(ice) && c1) {
+		/* only for SPDIF master mode, rate was changed */
+		rate = snd_ak4114_external_rate(ak4114);
+		/* printk(KERN_DEBUG "ak4114 - input rate changed to %d\n",
+				rate); */
+		juli_akm_set_rate_val(ice->akm, rate);
+	}
+}
+
 static int __devinit juli_init(struct snd_ice1712 *ice)
 {
 	static const unsigned char ak4114_init_vals[] = {
@@ -203,6 +597,11 @@ static int __devinit juli_init(struct sn
 				ice, &spec->ak4114);
 	if (err < 0)
 		return err;
+	/* callback for codecs rate setting */
+	spec->ak4114->change_callback = juli_ak4114_change;
+	spec->ak4114->change_callback_private = ice;
+	/* AK4114 in Juli can detect external rate correctly */
+	spec->ak4114->check_flags = 0;
 
 #if 0
         /* it seems that the analog doughter board detection does not work
@@ -226,6 +625,14 @@ static int __devinit juli_init(struct sn
 			return err;
 	}
 	
+	/* juli is clocked by Xilinx array */
+	ice->hw_rates = &juli_rates_info;
+	ice->is_spdif_master = juli_is_spdif_master;
+	ice->get_rate = juli_get_rate;
+	ice->set_rate = juli_set_rate;
+	ice->set_mclk = juli_set_mclk;
+	ice->set_spdif_clock = juli_set_spdif_clock;
+
 	ice->spdif.ops.open = juli_spdif_in_open;
 	return 0;
 }
@@ -237,18 +644,20 @@ static int __devinit juli_init(struct sn
  */
 
 static unsigned char juli_eeprom[] __devinitdata = {
-	[ICE_EEP2_SYSCONF]     = 0x20,	/* clock 512, mpu401, 1xADC, 1xDACs */
+	[ICE_EEP2_SYSCONF]     = 0x2b,	/* clock 512, mpu401, 1xADC, 1xDACs,
+					   SPDIF in */
 	[ICE_EEP2_ACLINK]      = 0x80,	/* I2S */
 	[ICE_EEP2_I2S]         = 0xf8,	/* vol, 96k, 24bit, 192k */
 	[ICE_EEP2_SPDIF]       = 0xc3,	/* out-en, out-int, spdif-in */
-	[ICE_EEP2_GPIO_DIR]    = 0x9f,
+	[ICE_EEP2_GPIO_DIR]    = 0x9f,	/* 5, 6:inputs; 7, 4-0 outputs*/
 	[ICE_EEP2_GPIO_DIR1]   = 0xff,
 	[ICE_EEP2_GPIO_DIR2]   = 0x7f,
-	[ICE_EEP2_GPIO_MASK]   = 0x9f,
-	[ICE_EEP2_GPIO_MASK1]  = 0xff,
+	[ICE_EEP2_GPIO_MASK]   = 0x60,	/* 5, 6: locked; 7, 4-0 writable */
+	[ICE_EEP2_GPIO_MASK1]  = 0x00,  /* 0-7 writable */
 	[ICE_EEP2_GPIO_MASK2]  = 0x7f,
-	[ICE_EEP2_GPIO_STATE]  = 0x16,	/* internal clock, multiple 1x, 48kHz */
-	[ICE_EEP2_GPIO_STATE1] = 0x80,	/* mute */
+	[ICE_EEP2_GPIO_STATE]  = GPIO_FREQ_48KHZ | GPIO_MULTI_1X |
+	       GPIO_INTERNAL_CLOCK,	/* internal clock, multiple 1x, 48kHz*/
+	[ICE_EEP2_GPIO_STATE1] = 0x00,	/* unmuted */
 	[ICE_EEP2_GPIO_STATE2] = 0x00,
 };

Re: PATCH - ESI Juli driver - removed debugs

[Pavel Hofman]

Takashi Iwai napsal(a):
> At Tue, 18 Mar 2008 07:56:11 +0100,
> Pavel Hofman wrote:
>> Takashi Iwai wrote:
>>> At Mon, 17 Mar 2008 16:50:39 +0100,
>>> Pavel Hofman wrote:
>>>
>>> Well, then we'll need to find some point to compromise.  Could you
>>> repost the patch after reducing the callbacks again?
>>>
>>> I'll have also no time from the next week at all -- will be on
>>> vacation for three weeks.  So, let's kick it out soonish :)
>>>
>> Hi,
>>
>> I am sorry, the previous patch had some debugs enabled. Here is a fixed 
>> patch.
>>
>> Thanks a lot.
>>
>> Pavel.
>>
>>
>> Signed-off-by: Pavel Hofman <dustin at seznam.cz>
>>
>> * moving most of clock-specific code to card-specific routines
>> * support for ESI Juli
>> * to-be-researched - monitoring of analog/digital inputs
> 
> Thanks.  Now I checked the patch and found it much better now.
> However, I still think the get_rate_index can be omitted by checking
> the index from the rate list dynamically.  This looks inefficent but
> improves the readability and reduces the code size better.  Moreover,
> the snd_pcm_hw's aren't needed to be prepared for each type.
> 
> Below is my revised patch.  With this patch, ice->hw_rates must be
> non-NULL and this is referred in many places.  It cleans up
> *_internal_clock*() nicely.
> 

Takashi, thanks a lot. I will test the patch tonight and let you know.

Pavel.

Re: PATCH - ESI Juli driver - removed debugs

[Pavel Hofman]

> 
> Takashi Iwai napsal(a):
>> At Tue, 18 Mar 2008 07:56:11 +0100,
>> Pavel Hofman wrote:
>>> Takashi Iwai wrote:
>>>> At Mon, 17 Mar 2008 16:50:39 +0100,
>>>> Pavel Hofman wrote:
>>>>
>>>> Well, then we'll need to find some point to compromise.  Could you
>>>> repost the patch after reducing the callbacks again?
>>>>
>>>> I'll have also no time from the next week at all -- will be on
>>>> vacation for three weeks.  So, let's kick it out soonish :)
>>>>
>>> Hi,
>>>
>>> I am sorry, the previous patch had some debugs enabled. Here is a fixed 
>>> patch.
>>>
>>> Thanks a lot.
>>>
>>> Pavel.
>>>
>>>
>>> Signed-off-by: Pavel Hofman <dustin at seznam.cz>
>>>
>>> * moving most of clock-specific code to card-specific routines
>>> * support for ESI Juli
>>> * to-be-researched - monitoring of analog/digital inputs
>> Thanks.  Now I checked the patch and found it much better now.
>> However, I still think the get_rate_index can be omitted by checking
>> the index from the rate list dynamically.  This looks inefficent but
>> improves the readability and reduces the code size better.  Moreover,
>> the snd_pcm_hw's aren't needed to be prepared for each type.
>>
>> Below is my revised patch.  With this patch, ice->hw_rates must be
>> non-NULL and this is referred in many places.  It cleans up
>> *_internal_clock*() nicely.
>>
> 

Takashi, thanks for commiting the patch. I just noticed the final patch 
does not change the native rates flags in snd_vt1724_playback_pro for 
juli, keeping the unsupported values 8000 - 12000 in the flags. I guess 
if 8kHz rate is played via plughw, plughw will not resample and juli 
will switch to gpios = 0, corresponding to 32kHz.

I can test in the evening, but this is very likely.

Pavel.

Re: PATCH - ESI Juli driver - OK

[Pavel Hofman]

Pavel Hofman wrote:

> 
> Takashi, thanks for commiting the patch. I just noticed the final patch 
> does not change the native rates flags in snd_vt1724_playback_pro for 
> juli, keeping the unsupported values 8000 - 12000 in the flags. I guess 
> if 8kHz rate is played via plughw, plughw will not resample and juli 
> will switch to gpios = 0, corresponding to 32kHz.
> 
> I can test in the evening, but this is very likely.
> 

The test proved the current version of the driver works correctly. 
plughw correctly oversampled input 8kHz signal to 16kHz on Juli, and 
played 8kHz directly on Prodigy192.

Thanks for the clean work :)

Pavel.

Re: PATCH - ESI Juli driver - works OK

[Pavel Hofman]

Takashi Iwai wrote:
> At Tue, 18 Mar 2008 07:56:11 +0100,
> Pavel Hofman wrote:
>> Takashi Iwai wrote:
>>> At Mon, 17 Mar 2008 16:50:39 +0100,
>>> Pavel Hofman wrote:
>>>
>>> Well, then we'll need to find some point to compromise.  Could you
>>> repost the patch after reducing the callbacks again?
>>>
>>> I'll have also no time from the next week at all -- will be on
>>> vacation for three weeks.  So, let's kick it out soonish :)
>>>
>> Hi,
>>
>> I am sorry, the previous patch had some debugs enabled. Here is a fixed 
>> patch.
>>
>> Thanks a lot.
>>
>> Pavel.
>>
>>
>> Signed-off-by: Pavel Hofman <dustin at seznam.cz>
>>
>> * moving most of clock-specific code to card-specific routines
>> * support for ESI Juli
>> * to-be-researched - monitoring of analog/digital inputs
> 
> Thanks.  Now I checked the patch and found it much better now.
> However, I still think the get_rate_index can be omitted by checking
> the index from the rate list dynamically.  This looks inefficent but
> improves the readability and reduces the code size better.  Moreover,
> the snd_pcm_hw's aren't needed to be prepared for each type.
> 
> Below is my revised patch.  With this patch, ice->hw_rates must be
> non-NULL and this is referred in many places.  It cleans up
> *_internal_clock*() nicely.
> 

Well Takashi, you are the real master here, your patch tells. I 
especially like the smart way you replaced the ugly switch/case 
structures in juli.c

The patch works fine, both standard clock as well as Juli.

Thanks a lot for your major help. Now time to move on to another audiobeast.

Pavel.

Re: PATCH - ESI Juli driver - works OK

[Takashi Iwai]

At Tue, 18 Mar 2008 20:24:40 +0100,
Pavel Hofman wrote:
> 
> Takashi Iwai wrote:
> > At Tue, 18 Mar 2008 07:56:11 +0100,
> > Pavel Hofman wrote:
> >> Takashi Iwai wrote:
> >>> At Mon, 17 Mar 2008 16:50:39 +0100,
> >>> Pavel Hofman wrote:
> >>>
> >>> Well, then we'll need to find some point to compromise.  Could you
> >>> repost the patch after reducing the callbacks again?
> >>>
> >>> I'll have also no time from the next week at all -- will be on
> >>> vacation for three weeks.  So, let's kick it out soonish :)
> >>>
> >> Hi,
> >>
> >> I am sorry, the previous patch had some debugs enabled. Here is a fixed 
> >> patch.
> >>
> >> Thanks a lot.
> >>
> >> Pavel.
> >>
> >>
> >> Signed-off-by: Pavel Hofman <dustin at seznam.cz>
> >>
> >> * moving most of clock-specific code to card-specific routines
> >> * support for ESI Juli
> >> * to-be-researched - monitoring of analog/digital inputs
> > 
> > Thanks.  Now I checked the patch and found it much better now.
> > However, I still think the get_rate_index can be omitted by checking
> > the index from the rate list dynamically.  This looks inefficent but
> > improves the readability and reduces the code size better.  Moreover,
> > the snd_pcm_hw's aren't needed to be prepared for each type.
> > 
> > Below is my revised patch.  With this patch, ice->hw_rates must be
> > non-NULL and this is referred in many places.  It cleans up
> > *_internal_clock*() nicely.
> > 
> 
> Well Takashi, you are the real master here, your patch tells. I 
> especially like the smart way you replaced the ugly switch/case 
> structures in juli.c
> 
> The patch works fine, both standard clock as well as Juli.

Thanks for confirming.  I applied the patch to HG tree now.


Takashi