В Wed, 20 Nov 2013 13:29:18 +0100 Clemens Ladisch clemens@ladisch.de пишет:
Roman Volkov wrote:
Clemens Ladisch clemens@ladisch.de пишет:
Roman Volkov wrote:
- 128kHz doesn't work for CS4361 (multichannel playback)
Why do you expect 128 kHz to work?
128kHz will not work because ...
... the CMI8788 does not support 128 kHz in the first place.
And perhaps my anti-pop delay of 1 second is too long.
The Windows driver uses 2.5 seconds.
Is the reset pin actually connected? Does the Windows driver do this?
Should be, otherwise this is bad design.
This driver is not for a theoretical, optimally designed card; it is for the actual card that Asus builds. If the Windows driver does not touch the reset pin, we should not do this either (because it might be connected to the self-destruct trigger).
sometimes (rarely) I get completely distorted sound when I power up the Windows machine.
After a cold boot?
This also happened for some other Xonar card when the Linux driver did things differently and did not reset these settings when shutting down.
Use mdelay() only when sleeping is not possible; use msleep() instead.
msleep() is not recommended for small delays (1ms-20ms).
Only because it isn't very accurate. But using mdelay() is even worse because it busy-loops.
- case PLAYBACK_DST_20_CH:
- case PLAYBACK_DST_40_CH:
- case PLAYBACK_DST_51_CH:
Why is it necessary to differentiate between those? There should be no mixer control; the driver can automatically detect what format is used. And this should not make any difference because the unused channels will be silent anyway.
Just as in the windows driver.
This is not necessarily a reason to implement the same in the Linux driver, especially if it's not required by the actual hardware, and reduces usability.
Should I change this to "HP\HP FP\Speakers"?
Yes.
+/* Put the codec into low power mode, disable audio output, save registers */
Does the Windows driver implement this?
I don't know. Linux docs says I should put my hardware into low power mode and I do that.
As far as I know, none of the Xonar cards have any real power-saving functionality; they rely on the entire PCI slot being powered down.
For practical purposes, suspend/resume just means that you have to restore the registers.
- .function_flags = OXYGEN_FUNCTION_SPI,
- .function_flags = OXYGEN_FUNCTION_SPI |
OXYGEN_FUNCTION_ENABLE_SPI_4_5,
Why?
Because this just works and as in the Windows driver.
If nothing is connected to those pins, it doesn't matter. This code in the Windows driver probably was just copy/pasted from the D2 code.
+++ linux-3.12-my/sound/pci/oxygen/xonar_dg.h
This file was intended as the interface between xonar_dg.c and the generic oxygen.c driver.
That you have to add so much stuff indicates that xonar_dg.c and xonar_dg_mixer.c are not very independent and maybe should have stayed a single file.
But moving mixer to another file so simplifies things...
Your choice. But it makes it hard to review changes when you move the code around in the same patch.
Regards, Clemens
Okay, I won't argue. If the driver will work with the changes, I don't care. How should I name the subject? Should I mark this with "RESEND" or "UPDATE" or leave the subject named like "[PATCH 2/2] snd-oxygen: Fixes for the Xonar DG card"?