diff mbox

ALSA: fix emu8000 DRAM sizing for AWE64 Value

Message ID 54AF259A.3070204@flaterco.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Flater Jan. 9, 2015, 12:49 a.m. UTC
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.
Also log the amount of memory detected at level INFO and add sufficiently
verbose debugging to diagnose any additional faults of this kind.

Tested on unexpanded CT4390 (4 MiB), CT4520 (512 KiB), and CT4380 (512 KiB).
The latter 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.

Signed-off-by: David Flater <dave@flaterco.com>
---
History:
2015-01-08  v1 patch sent to LKML, Alsa Devel and maintainers.

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.

AFAICT, the manufacturer never disclosed the right way to do it.  In the
AWE32 Developer's Information Pack (1994-1996) the RAM sizing function was
implemented in object files with a license that prohibited even disassembly.

 sound/isa/sb/emu8000.c | 127 +++++++++++++++++++++++++++++--------------------
 1 file changed, 76 insertions(+), 51 deletions(-)

Comments

Takashi Iwai Jan. 9, 2015, 11:09 a.m. UTC | #1
At Thu, 08 Jan 2015 19:49:30 -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.
> Also log the amount of memory detected at level INFO and add sufficiently
> verbose debugging to diagnose any additional faults of this kind.
> 
> Tested on unexpanded CT4390 (4 MiB), CT4520 (512 KiB), and CT4380 (512 KiB).
> The latter 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.
> 
> Signed-off-by: David Flater <dave@flaterco.com>
> ---
> History:
> 2015-01-08  v1 patch sent to LKML, Alsa Devel and maintainers.
> 
> 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.
> 
> AFAICT, the manufacturer never disclosed the right way to do it.  In the
> AWE32 Developer's Information Pack (1994-1996) the RAM sizing function was
> implemented in object files with a license that prohibited even disassembly.

Unfortunately it's not easy to read what you really changed because
the patch moves the loop in a deeper indentation.  Could you rewrite
to keep the original code as much as possible?  From what I read, the
necessary change would be something like:

	/* If that didn't work, we have no RAM at all */
	EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
	EMU8000_SMLD_READ(emu); /* discard stale data  */
	if (EMU8000_SMLD_READ(emu) != UNIQUE_ID1) {
		detected_size = 0;
		goto memory_detect_end;
	}

	snd_emu8000_read_wait(emu);
	detected_size = 512 * 1024;

	while (size < EMU8000_MAX_DRAM) {
		....
	}

memory_detect_end:
	emu->mem_size = detected_size;
 	emu->dram_checked = 1;

Also, don't change the printk level.  A patch should do only one
thing.  If you want to increase the printk level for the detected
memory size, do it another patch.


thanks,

Takashi

> 
>  sound/isa/sb/emu8000.c | 127 +++++++++++++++++++++++++++++--------------------
>  1 file changed, 76 insertions(+), 51 deletions(-)
> 
> diff --git a/sound/isa/sb/emu8000.c b/sound/isa/sb/emu8000.c
> index 45fcdff..3aa2250 100644
> --- a/sound/isa/sb/emu8000.c
> +++ b/sound/isa/sb/emu8000.c
> @@ -378,13 +378,13 @@ init_arrays(struct snd_emu8000 *emu)
>  static void
>  size_dram(struct snd_emu8000 *emu)
>  {
> -	int i, size, detected_size;
> +	int i, size;
> +	unsigned short rdback;
>  
>  	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,55 +392,81 @@ 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 */
> -
> -	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
> -		 * changed by this data.
> -		 */
> -		/*snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_WRITE);*/
> -		EMU8000_SMALW_WRITE(emu, EMU8000_DRAM_OFFSET + (size>>1));
> -		EMU8000_SMLD_WRITE(emu, UNIQUE_ID2);
> -		snd_emu8000_write_wait(emu);
> -
> -		/*
> -		 * read the data on the just written DRAM address
> -		 * if not the same then we have reached the end of ram.
> -		 */
> -		/*snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_READ);*/
> -		EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET + (size>>1));
> -		/*snd_emu8000_read_wait(emu);*/
> -		EMU8000_SMLD_READ(emu); /* discard stale data  */
> -		if (EMU8000_SMLD_READ(emu) != UNIQUE_ID2)
> -			break; /* no memory at this address */
> +	snd_emu8000_write_wait(emu);
> +
> +	/* If that didn't work, we have no RAM at all. */
> +	EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
> +	rdback = EMU8000_SMLD_READ(emu);
> +	snd_printdd("EMU8000 [0x%lx]: initial discard data = %04hx\n",
> +		emu->port1, rdback);
> +	rdback = EMU8000_SMLD_READ(emu);
> +	snd_printdd("EMU8000 [0x%lx]: initial readback = %04hx\n",
> +		emu->port1, rdback);
> +	if (rdback == UNIQUE_ID1) {
>  		snd_emu8000_read_wait(emu);
>  
>  		/*
> -		 * If it is the same it could be that the address just
> -		 * wraps back to the beginning; so check to see if the
> -		 * initial value has been overwritten.
> +		 * If a write succeeds at the beginning of a 512 KiB page we
> +		 * assume that the whole page is there.
>  		 */
> -		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 */
> -		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;
> +		size = 512 * 1024;
> +
> +		while (size < EMU8000_MAX_DRAM) {
> +			/*
> +			 * 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 changed by this data.
> +			 */
> +			EMU8000_SMALW_WRITE(emu, EMU8000_DRAM_OFFSET +
> +				(size>>1));
> +			EMU8000_SMLD_WRITE(emu, UNIQUE_ID2);
> +			snd_emu8000_write_wait(emu);
> +
> +			/*
> +			 * Read the data on the just written DRAM address.
> +			 * If not the same then we have reached the end of RAM.
> +			 */
> +			EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET +
> +				(size>>1));
> +			rdback = EMU8000_SMLD_READ(emu);
> +			snd_printdd("EMU8000 [0x%lx]: ID2 discard data = %04hx\n",
> +				emu->port1, rdback);
> +			rdback = EMU8000_SMLD_READ(emu);
> +			snd_printdd("EMU8000 [0x%lx]: ID2 readback = %04hx\n",
> +				emu->port1, rdback);
> +			if (rdback != UNIQUE_ID2) {
> +				snd_printdd("EMU8000 [0x%lx]: ID2 break\n",
> +					emu->port1);
> +				break; /* no memory at this address */
> +			}
> +			snd_emu8000_read_wait(emu);
> +
> +			/*
> +			 * If it is the same it could be that the address just
> +			 * wraps back to the beginning, so check to see if the
> +			 * initial value has been overwritten.
> +			 */
> +			EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
> +			rdback = EMU8000_SMLD_READ(emu);
> +			snd_printdd("EMU8000 [0x%lx]: ID1 discard data = %04hx\n",
> +				emu->port1, rdback);
> +			rdback = EMU8000_SMLD_READ(emu);
> +			snd_printdd("EMU8000 [0x%lx]: ID1 readback = %04hx\n",
> +				emu->port1, rdback);
> +			if (rdback != UNIQUE_ID1) {
> +				snd_printdd("EMU8000 [0x%lx]: ID1 break\n",
> +					emu->port1);
> +				break; /* may have wrapped around */
> +			}
> +			snd_emu8000_read_wait(emu);
> +
> +			/* Otherwise, it's valid memory. */
> +			size += 512 * 1024;
> +		}
> +	} else {
> +		snd_printdd("EMU8000 [0x%lx]: initial test failed\n",
> +			emu->port1);
>  	}
>  
>  	/* wait until FULL bit in SMAxW register is false */
> @@ -454,10 +480,9 @@ size_dram(struct snd_emu8000 *emu)
>  	snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_CLOSE);
>  	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->mem_size = detected_size;
> +	snd_printk(KERN_INFO "sbawe [0x%lx]: %d B on-board DRAM detected\n",
> +		emu->port1, size);
> +	emu->mem_size = size;
>  	emu->dram_checked = 1;
>  }
>  
> -- 
> 1.8.4
>
diff mbox

Patch

diff --git a/sound/isa/sb/emu8000.c b/sound/isa/sb/emu8000.c
index 45fcdff..3aa2250 100644
--- a/sound/isa/sb/emu8000.c
+++ b/sound/isa/sb/emu8000.c
@@ -378,13 +378,13 @@  init_arrays(struct snd_emu8000 *emu)
 static void
 size_dram(struct snd_emu8000 *emu)
 {
-	int i, size, detected_size;
+	int i, size;
+	unsigned short rdback;
 
 	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,55 +392,81 @@  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 */
-
-	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
-		 * changed by this data.
-		 */
-		/*snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_WRITE);*/
-		EMU8000_SMALW_WRITE(emu, EMU8000_DRAM_OFFSET + (size>>1));
-		EMU8000_SMLD_WRITE(emu, UNIQUE_ID2);
-		snd_emu8000_write_wait(emu);
-
-		/*
-		 * read the data on the just written DRAM address
-		 * if not the same then we have reached the end of ram.
-		 */
-		/*snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_READ);*/
-		EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET + (size>>1));
-		/*snd_emu8000_read_wait(emu);*/
-		EMU8000_SMLD_READ(emu); /* discard stale data  */
-		if (EMU8000_SMLD_READ(emu) != UNIQUE_ID2)
-			break; /* no memory at this address */
+	snd_emu8000_write_wait(emu);
+
+	/* If that didn't work, we have no RAM at all. */
+	EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
+	rdback = EMU8000_SMLD_READ(emu);
+	snd_printdd("EMU8000 [0x%lx]: initial discard data = %04hx\n",
+		emu->port1, rdback);
+	rdback = EMU8000_SMLD_READ(emu);
+	snd_printdd("EMU8000 [0x%lx]: initial readback = %04hx\n",
+		emu->port1, rdback);
+	if (rdback == UNIQUE_ID1) {
 		snd_emu8000_read_wait(emu);
 
 		/*
-		 * If it is the same it could be that the address just
-		 * wraps back to the beginning; so check to see if the
-		 * initial value has been overwritten.
+		 * If a write succeeds at the beginning of a 512 KiB page we
+		 * assume that the whole page is there.
 		 */
-		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 */
-		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;
+		size = 512 * 1024;
+
+		while (size < EMU8000_MAX_DRAM) {
+			/*
+			 * 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 changed by this data.
+			 */
+			EMU8000_SMALW_WRITE(emu, EMU8000_DRAM_OFFSET +
+				(size>>1));
+			EMU8000_SMLD_WRITE(emu, UNIQUE_ID2);
+			snd_emu8000_write_wait(emu);
+
+			/*
+			 * Read the data on the just written DRAM address.
+			 * If not the same then we have reached the end of RAM.
+			 */
+			EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET +
+				(size>>1));
+			rdback = EMU8000_SMLD_READ(emu);
+			snd_printdd("EMU8000 [0x%lx]: ID2 discard data = %04hx\n",
+				emu->port1, rdback);
+			rdback = EMU8000_SMLD_READ(emu);
+			snd_printdd("EMU8000 [0x%lx]: ID2 readback = %04hx\n",
+				emu->port1, rdback);
+			if (rdback != UNIQUE_ID2) {
+				snd_printdd("EMU8000 [0x%lx]: ID2 break\n",
+					emu->port1);
+				break; /* no memory at this address */
+			}
+			snd_emu8000_read_wait(emu);
+
+			/*
+			 * If it is the same it could be that the address just
+			 * wraps back to the beginning, so check to see if the
+			 * initial value has been overwritten.
+			 */
+			EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
+			rdback = EMU8000_SMLD_READ(emu);
+			snd_printdd("EMU8000 [0x%lx]: ID1 discard data = %04hx\n",
+				emu->port1, rdback);
+			rdback = EMU8000_SMLD_READ(emu);
+			snd_printdd("EMU8000 [0x%lx]: ID1 readback = %04hx\n",
+				emu->port1, rdback);
+			if (rdback != UNIQUE_ID1) {
+				snd_printdd("EMU8000 [0x%lx]: ID1 break\n",
+					emu->port1);
+				break; /* may have wrapped around */
+			}
+			snd_emu8000_read_wait(emu);
+
+			/* Otherwise, it's valid memory. */
+			size += 512 * 1024;
+		}
+	} else {
+		snd_printdd("EMU8000 [0x%lx]: initial test failed\n",
+			emu->port1);
 	}
 
 	/* wait until FULL bit in SMAxW register is false */
@@ -454,10 +480,9 @@  size_dram(struct snd_emu8000 *emu)
 	snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_CLOSE);
 	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->mem_size = detected_size;
+	snd_printk(KERN_INFO "sbawe [0x%lx]: %d B on-board DRAM detected\n",
+		emu->port1, size);
+	emu->mem_size = size;
 	emu->dram_checked = 1;
 }