From patchwork Sun Jan 11 10:52:17 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 5605721 Return-Path: X-Original-To: patchwork-alsa-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5439B9F358 for ; Sun, 11 Jan 2015 10:52:38 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 552872056E for ; Sun, 11 Jan 2015 10:52:37 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) by mail.kernel.org (Postfix) with ESMTP id 932AD20567 for ; Sun, 11 Jan 2015 10:52:35 +0000 (UTC) Received: by alsa0.perex.cz (Postfix, from userid 1000) id 20444260502; Sun, 11 Jan 2015 11:52:34 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from alsa0.perex.cz (localhost [IPv6:::1]) by alsa0.perex.cz (Postfix) with ESMTP id AFAB72604C3; Sun, 11 Jan 2015 11:52:25 +0100 (CET) X-Original-To: alsa-devel@alsa-project.org Delivered-To: alsa-devel@alsa-project.org Received: by alsa0.perex.cz (Postfix, from userid 1000) id 038442604C7; Sun, 11 Jan 2015 11:52:24 +0100 (CET) Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 15B092604C3 for ; Sun, 11 Jan 2015 11:52:18 +0100 (CET) Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id B59BBAB0E; Sun, 11 Jan 2015 10:52:17 +0000 (UTC) Date: Sun, 11 Jan 2015 11:52:17 +0100 Message-ID: From: Takashi Iwai To: David Flater In-Reply-To: <54B0775C.1080705@flaterco.com> References: <54B0775C.1080705@flaterco.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.4 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Cc: Alsa Devel , LKML Subject: Re: [alsa-devel] [PATCHv2 1/2] ALSA: fix emu8000 DRAM sizing for AWE64 Value X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org X-Virus-Scanned: ClamAV using ClamSMTP At Fri, 09 Jan 2015 19:50:36 -0500, David Flater wrote: > > Applicable to any kernel since 2013: > > The special case added in commit 1338fc97d07a did not handle the possibility > that the address space on an AWE64 Value would wrap around at 512 KiB. That > is what it does, so the memory is still not detected on those cards. > > Fix that with a logic clean-up that eliminates the need for a special case. > > Signed-off-by: David Flater > --- > History: > 2015-01-09 v2: In response to feedback from Takashi Iwai, > 1. Optimize for diff size. > 1a. Use goto to avoid indenting and repeating code. > 1b. Jettison new debugging printouts. > 2. Split printk into second patch. > Retested on CT4390 (4 MiB) and CT4380 (512 KiB). > 2015-01-08 v1 patch sent to LKML, Alsa Devel and maintainers. Tested on > unexpanded CT4390 (4 MiB), CT4520 (512 KiB), and CT4380 (512 > KiB). > > CT4380 is commonly said to come with 1 MiB of DRAM, but Creative's AWE > Control app agreed that mine has only 512 KiB. It has the same memory chip > as the CT4520. And you couldn't actually use the 1MB, right? > The affected function first appeared in alsa-driver-0.3.0 and was merged in > linux-2.5.5. Its somewhat different ancestor was in sound/oss/awe_wave.c. > > sound/isa/sb/emu8000.c | 33 +++++++++++++++------------------ > 1 file changed, 15 insertions(+), 18 deletions(-) > > diff --git a/sound/isa/sb/emu8000.c b/sound/isa/sb/emu8000.c > index 45fcdff..3dcf80e 100644 > --- a/sound/isa/sb/emu8000.c > +++ b/sound/isa/sb/emu8000.c > @@ -378,13 +378,12 @@ init_arrays(struct snd_emu8000 *emu) > static void > size_dram(struct snd_emu8000 *emu) > { > - int i, size, detected_size; > + int i, size; > > if (emu->dram_checked) > return; > > size = 0; > - detected_size = 0; > > /* write out a magic number */ > snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_WRITE); > @@ -392,11 +391,12 @@ size_dram(struct snd_emu8000 *emu) > EMU8000_SMALW_WRITE(emu, EMU8000_DRAM_OFFSET); > EMU8000_SMLD_WRITE(emu, UNIQUE_ID1); > snd_emu8000_init_fm(emu); /* This must really be here and not 2 lines back even */ > + snd_emu8000_write_wait(emu); > + > + goto size_dram_ID1_check; /* If that fails, we have no RAM. */ Jumping into the middle of the loop isn't good. It worsens the readability a lot. OTOH, jumping to the exit of the loop is a standard idiom like the patch below. thanks, Takashi diff --git a/sound/isa/sb/emu8000.c b/sound/isa/sb/emu8000.c index 45fcdff611f9..6e721a8b7c9e 100644 --- a/sound/isa/sb/emu8000.c +++ b/sound/isa/sb/emu8000.c @@ -378,13 +378,12 @@ init_arrays(struct snd_emu8000 *emu) static void size_dram(struct snd_emu8000 *emu) { - int i, size, detected_size; + int i, size; if (emu->dram_checked) return; size = 0; - detected_size = 0; /* write out a magic number */ snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_WRITE); @@ -393,9 +392,14 @@ size_dram(struct snd_emu8000 *emu) EMU8000_SMLD_WRITE(emu, UNIQUE_ID1); snd_emu8000_init_fm(emu); /* This must really be here and not 2 lines back even */ - while (size < EMU8000_MAX_DRAM) { + /* Detect first 512 KiB */ + snd_emu8000_write_wait(emu); + EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET); + EMU8000_SMLD_READ(emu); /* discard stale data */ + if (EMU8000_SMLD_READ(emu) == UNIQUE_ID1) + goto skip_detect; - size += 512 * 1024; /* increment 512kbytes */ + for (size = 512 * 1024; size < EMU8000_MAX_DRAM; size += 512 * 1024) { /* Write a unique data on the test address. * if the address is out of range, the data is written on @@ -429,20 +433,9 @@ size_dram(struct snd_emu8000 *emu) if (EMU8000_SMLD_READ(emu) != UNIQUE_ID1) break; /* we must have wrapped around */ snd_emu8000_read_wait(emu); - - /* Otherwise, it's valid memory. */ - detected_size = size + 512 * 1024; - } - - /* Distinguish 512 KiB from 0. */ - if (detected_size == 0) { - snd_emu8000_read_wait(emu); - EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET); - EMU8000_SMLD_READ(emu); /* discard stale data */ - if (EMU8000_SMLD_READ(emu) == UNIQUE_ID1) - detected_size = 512 * 1024; } + skip_detect: /* wait until FULL bit in SMAxW register is false */ for (i = 0; i < 10000; i++) { if ((EMU8000_SMALW_READ(emu) & 0x80000000) == 0) @@ -455,9 +448,9 @@ size_dram(struct snd_emu8000 *emu) snd_emu8000_dma_chan(emu, 1, EMU8000_RAM_CLOSE); snd_printdd("EMU8000 [0x%lx]: %d Kb on-board memory detected\n", - emu->port1, detected_size/1024); + emu->port1, size / 1024); - emu->mem_size = detected_size; + emu->mem_size = size; emu->dram_checked = 1; }