* [PATCHv2 1/2] ALSA: fix emu8000 DRAM sizing for AWE64 Value
@ 2015-01-10 0:50 David Flater
2015-01-11 10:52 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: David Flater @ 2015-01-10 0:50 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Jaroslav Kysela, LKML, Alsa Devel
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 <dave@flaterco.com>
---
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.
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. */
while (size < EMU8000_MAX_DRAM) {
- size += 512 * 1024; /* increment 512kbytes */
-
/* Write a unique data on the test address.
* if the address is out of range, the data is written on
* 0x200000(=EMU8000_DRAM_OFFSET). Then the id word is
@@ -424,23 +424,20 @@ size_dram(struct snd_emu8000 *emu)
* wraps back to the beginning; so check to see if the
* initial value has been overwritten.
*/
+
+ size_dram_ID1_check:
EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
EMU8000_SMLD_READ(emu); /* discard stale data */
if (EMU8000_SMLD_READ(emu) != UNIQUE_ID1)
- break; /* we must have wrapped around */
+ break;
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;
+ /*
+ * Otherwise, it's valid memory. If a write succeeds at
+ * the beginning of a 512 KiB page we assume that the whole
+ * page is there.
+ */
+ size += 512 * 1024; /* increment 512 KiB */
}
/* wait until FULL bit in SMAxW register is false */
@@ -455,9 +452,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;
}
--
1.8.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCHv2 1/2] ALSA: fix emu8000 DRAM sizing for AWE64 Value
2015-01-10 0:50 [PATCHv2 1/2] ALSA: fix emu8000 DRAM sizing for AWE64 Value David Flater
@ 2015-01-11 10:52 ` Takashi Iwai
0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2015-01-11 10:52 UTC (permalink / raw)
To: David Flater; +Cc: Jaroslav Kysela, LKML, Alsa Devel
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 <dave@flaterco.com>
> ---
> 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;
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCHv2 1/2] ALSA: fix emu8000 DRAM sizing for AWE64 Value
@ 2015-01-11 10:52 ` Takashi Iwai
0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2015-01-11 10:52 UTC (permalink / raw)
To: David Flater; +Cc: Alsa Devel, LKML
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 <dave@flaterco.com>
> ---
> 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;
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCHv2 1/2] ALSA: fix emu8000 DRAM sizing for AWE64 Value
2015-01-11 10:52 ` Takashi Iwai
(?)
@ 2015-01-11 17:10 ` David Flater
-1 siblings, 0 replies; 4+ messages in thread
From: David Flater @ 2015-01-11 17:10 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Jaroslav Kysela, LKML, Alsa Devel
On 01/11/2015 05:52 AM, Takashi Iwai wrote:
> At Fri, 09 Jan 2015 19:50:36 -0500,
> David Flater wrote:
>> 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?
Confirmed with indirect evidence. Since the address space wraps around, the mode of failure (assuming that the load isn't blocked by checking against a correctly detected size) is silent corruption of the loaded sound font.
Testing with a random sound font larger than 512 KiB but smaller than 1 MiB:
Fender2.sf2, 545994 B
On CT4390, the following test gives me a credible guitar in the left channel (and nothing else):
asfxload -i -M Fender2.sf2
DRAM memory left = 3565 kB
aplaymidi -p 17:0 E1M1.mid
On CT4380 with DRAM size hard-coded to 1 MiB, asfxload catches no failure but aplaymidi produces a glitchy mixture of guitar and morse code beeping:
asfxload -i -M Fender2.sf2
DRAM memory left = 493 kB
aplaymidi -p 17:0 E1M1.mid
DWF
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-01-11 17:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-10 0:50 [PATCHv2 1/2] ALSA: fix emu8000 DRAM sizing for AWE64 Value David Flater
2015-01-11 10:52 ` Takashi Iwai
2015-01-11 10:52 ` Takashi Iwai
2015-01-11 17:10 ` David Flater
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.