[alsa-devel] [PATCH 2/2] snd-oxygen: Fixes for the Xonar DG card
v1ron at mail.ru
Wed Nov 20 14:15:23 CET 2013
В Wed, 20 Nov 2013 13:29:18 +0100
Clemens Ladisch <clemens at ladisch.de> пишет:
> Roman Volkov wrote:
> > Clemens Ladisch <clemens at 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
> >> 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"?
> >>> +/* 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.
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"?
v1ron at mail.ru
More information about the Alsa-devel