public inbox for linux-sound@vger.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: "Vittorio Gambaletta (VittGam)" <linuxbugs@vittgam.net>
Cc: linux-sound@vger.kernel.org, alsa-devel@alsa-project.org,
	stable@vger.kernel.org, Jaroslav Kysela <perex@perex.cz>
Subject: Re: [PATCH] ALSA: intel8x0: Add clock quirk entry for AD1981B on IBM ThinkPad X41.
Date: Mon, 14 Mar 2016 07:30:29 +0000	[thread overview]
Message-ID: <s5hegbdy0ru.wl-tiwai@suse.de> (raw)
In-Reply-To: <linux-patch-20160313-1@vittgam.net>

On Sun, 13 Mar 2016 22:19:34 +0100,
Vittorio Gambaletta (VittGam) wrote:
> 
> The clock measurement on the AC'97 audio card found in the IBM ThinkPad X41
> will often fail, so add a quirk entry to fix it.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?idD1087
> Cc: <stable@vger.kernel.org>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Vittorio Gambaletta <linuxbugs@vittgam.net>
> ---
> 
> When the clock measurement doesn't fail, the `pos` variable after line 2861
> will be around 48367, but when it does fail it will be somewhere between
> 45000 and 47500. So the bus clock gets set between 48500 and 52000 (!),
> resulting in faster audio for rates with final clocks below 48000, and
> distorted audio for rates with final clocks above 48000.
> 
> The problem is always reproducible on kernel 4.4.5, but very rarely on 3.13.

Hrm, does this mean that the ktime() isn't accurate enough, or do we
sample too short time?  i.e. Is it improved if we enlarge the
measurement time?

> This patch fixes the problem for my computer, and will also speed up module
> loading a bit as said on Bugzilla, but I've got some questions regarding
> other hardware...
> 
> Do we want to protect against overclock for all hardware, by never setting
> the measured bus clock higher than 48000? Do we actually risk burning or
> otherwise permanently ruining something here, or not? Does hardware clocked
> above 48000 exist?
> 
> Also, is line 2874 actually correct? As `pos` gets higher, the bus clock gets
> lower, and vice versa. Note that in this function the bus clock is always
> 48000 before it gets updated here; so the code first computes 48000 * 48000,
> and then it divides the result by `pos`. It doesn't seem right to me... By the
> way, the history of that line goes beyond Git (so before kernel 2.6.12-rc2).

I don't remember of any machines with clock > 48k, so I guess the code
never hits on any real machines, and I'm not sure whether it's really
correct at a quick glance.

In anyway, I applied your patch now.  Thanks.


Takashi

      reply	other threads:[~2016-03-14  7:30 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-13 21:19 [PATCH] ALSA: intel8x0: Add clock quirk entry for AD1981B on IBM ThinkPad X41 Vittorio Gambaletta (VittGam)
2016-03-14  7:30 ` Takashi Iwai [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hegbdy0ru.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=linuxbugs@vittgam.net \
    --cc=perex@perex.cz \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox