Skip to contentSkip to conversation

alsa-kernel clean-ups

RFC In review Takashi Iwai 3 participants 17 messages Dec 20, 2007 – Jan 8, 2008 Documentation

lore
Share & tools
lore

Takashi Iwai

author
#0 lore raw

Hi,

ALSA kernel tree has lots of unused and rotten codes. I'd like to clean up this. Here we go...

1. Indirect control element access

This is what I already suggested. This hasn't been used (we have no proper API in alsa-lib), and 32/64bit wrapper doesn't work at all.

Also, the idea to copy such a big data area via ioctl is bad. If we need a big matrix mixer, let make each matrix element accessible individually. Copying the whole matrix at each time isn't efficient.

2. PCM xfer_align parameter

This sw_params parameter has never been used in a sane manner, and no one understands what this does exactly. The current implementation looks also buggy because it allows write of shorter size than xfer_align. So, if you do partial writes, the write isn't actually aligned at all.

Removing this parameter will make some pcm_lib_* code more readable (and less buggy).

3. PCM sleep_min and tick

The "tick" in PCM is set (again) via sw_params. And, nobody uses this feature at all except for a command line option of aplay. (This is literally "nobody", as I checked alsa-lib API calls in all programs in some major distros.)

Above all, if we need finer wake-ups for the position update, it's basically an issue that the driver should solve, not tuned by each application.

4. sound/driver.h

This header file exists only for some hacks to adapt alsa-driver tree. It's useless for building in the kernel. Let's move a few lines in it to sound/core.h and remove it.

For building the modules on alsa-driver external tree, we can simply add

    #include "adriver.h"

to each build stub (alsa-driver/*/*.c) before inclusion of alsa-kernel codes.

I'll post some patches to clean the above thing up in the following posts. Please speak up if you have objections.

Takashi

author
#1 lore raw

RFC 1/1 Remove indirect control access

Hi,

this is the patch to remove indirect control access.
In this version, I left the pointers in snd_ctl_elem_* struct but added/changed comments because the pointer member might influence on alignment. After confirming that it doesn't affect, we can get rid of them.

Takashi

core/control.c +20 -31
diff -r 83ddcb1104e6 core/control.c
@@ -232,8 +232,6 @@ struct snd_kcontrol *snd_ctl_new1(const 
 	access = ncontrol->access == 0 ? SNDRV_CTL_ELEM_ACCESS_READWRITE :
 		 (ncontrol->access & (SNDRV_CTL_ELEM_ACCESS_READWRITE|
 				      SNDRV_CTL_ELEM_ACCESS_INACTIVE|
-		 		      SNDRV_CTL_ELEM_ACCESS_DINDIRECT|
-		 		      SNDRV_CTL_ELEM_ACCESS_INDIRECT|
 		 		      SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE|
 		 		      SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK));
 	kctl.info = ncontrol->info;

@@ -692,7 +690,7 @@ int snd_ctl_elem_read(struct snd_card *c
 	struct snd_kcontrol *kctl;
 	struct snd_kcontrol_volatile *vd;
 	unsigned int index_offset;
-	int result, indirect;
+	int result;
 
 	down_read(&card->controls_rwsem);
 	kctl = snd_ctl_find_id(card, &control->id);

@@ -701,17 +699,12 @@ int snd_ctl_elem_read(struct snd_card *c
 	} else {
 		index_offset = snd_ctl_get_ioff(kctl, &control->id);
 		vd = &kctl->vd[index_offset];
-		indirect = vd->access & SNDRV_CTL_ELEM_ACCESS_INDIRECT ? 1 : 0;
-		if (control->indirect != indirect) {
-			result = -EACCES;
-		} else {
-			if ((vd->access & SNDRV_CTL_ELEM_ACCESS_READ) && kctl->get != NULL) {
-				snd_ctl_build_ioff(&control->id, kctl, index_offset);
-				result = kctl->get(kctl, control);
-			} else {
-				result = -EPERM;
-			}
-		}
+		if ((vd->access & SNDRV_CTL_ELEM_ACCESS_READ) &&
+		    kctl->get != NULL) {
+			snd_ctl_build_ioff(&control->id, kctl, index_offset);
+			result = kctl->get(kctl, control);
+		} else
+			result = -EPERM;
 	}
 	up_read(&card->controls_rwsem);
 	return result;

@@ -748,7 +741,7 @@ int snd_ctl_elem_write(struct snd_card *
 	struct snd_kcontrol *kctl;
 	struct snd_kcontrol_volatile *vd;
 	unsigned int index_offset;
-	int result, indirect;
+	int result;
 
 	down_read(&card->controls_rwsem);
 	kctl = snd_ctl_find_id(card, &control->id);

@@ -757,23 +750,19 @@ int snd_ctl_elem_write(struct snd_card *
 	} else {
 		index_offset = snd_ctl_get_ioff(kctl, &control->id);
 		vd = &kctl->vd[index_offset];
-		indirect = vd->access & SNDRV_CTL_ELEM_ACCESS_INDIRECT ? 1 : 0;
-		if (control->indirect != indirect) {
-			result = -EACCES;
+		if (!(vd->access & SNDRV_CTL_ELEM_ACCESS_WRITE) ||
+		    kctl->put == NULL ||
+		    (file && vd->owner && vd->owner != file)) {
+			result = -EPERM;
 		} else {
-			if (!(vd->access & SNDRV_CTL_ELEM_ACCESS_WRITE) ||
-			    kctl->put == NULL ||
-			    (file && vd->owner != NULL && vd->owner != file)) {
-				result = -EPERM;
-			} else {
-				snd_ctl_build_ioff(&control->id, kctl, index_offset);
-				result = kctl->put(kctl, control);
-			}
-			if (result > 0) {
-				up_read(&card->controls_rwsem);
-				snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &control->id);
-				return 0;
-			}
+			snd_ctl_build_ioff(&control->id, kctl, index_offset);
+			result = kctl->put(kctl, control);
+		}
+		if (result > 0) {
+			up_read(&card->controls_rwsem);
+			snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE,
+				       &control->id);
+			return 0;
 		}
 	}
 	up_read(&card->controls_rwsem);
include/asound.h +7 -8
diff -r 83ddcb1104e6 include/asound.h
@@ -745,8 +745,7 @@ typedef int __bitwise snd_ctl_elem_iface
 #define SNDRV_CTL_ELEM_ACCESS_OWNER		(1<<10)	/* write lock owner */
 #define SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK	(1<<28)	/* kernel use a TLV callback */ 
 #define SNDRV_CTL_ELEM_ACCESS_USER		(1<<29) /* user space element */
-#define SNDRV_CTL_ELEM_ACCESS_DINDIRECT		(1<<30)	/* indirect access for matrix dimensions in the info structure */
-#define SNDRV_CTL_ELEM_ACCESS_INDIRECT		(1<<31)	/* indirect access for element value in the value structure */
+/* bits 30 and 31 are obsoleted (for indirect access) */
 
 /* for further details see the ACPI and PCI power management specification */
 #define SNDRV_CTL_POWER_D0		0x0000	/* full On */

@@ -800,30 +799,30 @@ struct snd_ctl_elem_info {
 	} value;
 	union {
 		unsigned short d[4];		/* dimensions */
-		unsigned short *d_ptr;		/* indirect */
+		unsigned short *d_ptr;		/* indirect - obsoleted */
 	} dimen;
 	unsigned char reserved[64-4*sizeof(unsigned short)];
 };
 
 struct snd_ctl_elem_value {
 	struct snd_ctl_elem_id id;	/* W: element ID */
-	unsigned int indirect: 1;	/* W: use indirect pointer (xxx_ptr member) */
+	unsigned int indirect: 1;	/* W: indirect access - obsoleted */
         union {
 		union {
 			long value[128];
-			long *value_ptr;
+			long *value_ptr;	/* obsoleted */
 		} integer;
 		union {
 			long long value[64];
-			long long *value_ptr;
+			long long *value_ptr;	/* obsoleted */
 		} integer64;
 		union {
 			unsigned int item[128];
-			unsigned int *item_ptr;
+			unsigned int *item_ptr;	/* obsoleted */
 		} enumerated;
 		union {
 			unsigned char data[512];
-			unsigned char *data_ptr;
+			unsigned char *data_ptr;	/* obsoleted */
 		} bytes;
 		struct snd_aes_iec958 iec958;
         } value;                /* RO */
#2 lore raw

Re: [RFC] Remove indirect control access

Quoted 7 lines · Takashi Iwai

Hi,

this is the patch to remove indirect control access.
In this version, I left the pointers in snd_ctl_elem_* struct but added/changed comments because the pointer member might influence on alignment. After confirming that it doesn't affect, we can get rid of them.

Ack, but I would increase also protocol version number to have a possibility to check the kernel API change in alsa-lib.

Jaroslav

-----
Jaroslav Kysela <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

author
#3 lore raw

Re: [RFC] Remove indirect control access

At Tue, 8 Jan 2008 17:32:29 +0100 (CET),

Quoted 5 lines · Jaroslav Kysela
Quoted 2 lines · Takashi Iwai

Hi,

Quoted 8 lines
Quoted 5 lines

this is the patch to remove indirect control access.
In this version, I left the pointers in snd_ctl_elem_* struct but added/changed comments because the pointer member might influence on alignment. After confirming that it doesn't affect, we can get rid of them.

Ack, but I would increase also protocol version number to have a possibility to check the kernel API change in alsa-lib.

OK, will do.

Takashi

author
#4 lore raw

RFC 1/1 Remove PCM xfer_align parameter

In reply to Takashi Iwai

This patch removes the PCM xfer_align parameter. It's still present and won't break the kernel ABI.

Note that this patch won't be applied cleanly to HG, as it's generated in the middle of my patch stack (to fix snd_pcm_lib_write1 blocking bug).

Takashi

Documentation/DocBook/writing-an-alsa-driver.tmpl +0 -1
diff -r 3caa0db34608 Documentation/DocBook/writing-an-alsa-driver.tmpl
@@ -2185,7 +2185,6 @@ struct _snd_pcm_runtime {
 	struct timespec tstamp_mode;	/* mmap timestamp is updated */
   	unsigned int period_step;
 	unsigned int sleep_min;		/* min ticks to sleep */
-	snd_pcm_uframes_t xfer_align;	/* xfer size need to be a multiple */
 	snd_pcm_uframes_t start_threshold;
 	snd_pcm_uframes_t stop_threshold;
 	snd_pcm_uframes_t silence_threshold; /* Silence filling happens when
core/oss/pcm_oss.c +0 -1
diff -r 3caa0db34608 core/oss/pcm_oss.c
@@ -988,7 +988,6 @@ static int snd_pcm_oss_change_params(str
 	sw_params->sleep_min = 0;
 	sw_params->avail_min = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ?
 		1 : runtime->period_size;
-	sw_params->xfer_align = 1;
 	if (atomic_read(&substream->mmap_count) ||
 	    substream->oss.setup.nosilence) {
 		sw_params->silence_threshold = 0;
core/pcm.c +0 -1
diff -r 3caa0db34608 core/pcm.c
@@ -389,7 +389,6 @@ static void snd_pcm_substream_proc_sw_pa
 	snd_iprintf(buffer, "period_step: %u\n", runtime->period_step);
 	snd_iprintf(buffer, "sleep_min: %u\n", runtime->sleep_min);
 	snd_iprintf(buffer, "avail_min: %lu\n", runtime->control->avail_min);
-	snd_iprintf(buffer, "xfer_align: %lu\n", runtime->xfer_align);
 	snd_iprintf(buffer, "start_threshold: %lu\n", runtime->start_threshold);
 	snd_iprintf(buffer, "stop_threshold: %lu\n", runtime->stop_threshold);
 	snd_iprintf(buffer, "silence_threshold: %lu\n", runtime->silence_threshold);
core/pcm_lib.c +4 -14
diff -r 3caa0db34608 core/pcm_lib.c
@@ -1598,8 +1598,6 @@ static snd_pcm_sframes_t snd_pcm_lib_wri
 
 	if (size == 0)
 		return 0;
-	if (size > runtime->xfer_align)
-		size -= size % runtime->xfer_align;
 
 	snd_pcm_stream_lock_irq(substream);
 	switch (runtime->status->state) {

@@ -1627,9 +1625,7 @@ static snd_pcm_sframes_t snd_pcm_lib_wri
 		avail = snd_pcm_playback_avail(runtime);
 		if (!avail ||
 		    (snd_pcm_running(substream) &&
-		     ((avail < runtime->control->avail_min && size > avail) ||
-		      (size >= runtime->xfer_align &&
-		       avail < runtime->xfer_align)))) {
+		     (avail < runtime->control->avail_min && size > avail))) {
 			wait_queue_t wait;
 			enum { READY, SIGNALED, ERROR, SUSPENDED, EXPIRED, DROPPED } state;
 			long tout;

@@ -1701,8 +1697,6 @@ static snd_pcm_sframes_t snd_pcm_lib_wri
 				break;
 			}
 		}
-		if (avail > runtime->xfer_align)
-			avail -= avail % runtime->xfer_align;
 		frames = size > avail ? avail : size;
 		cont = runtime->buffer_size - runtime->control->appl_ptr % runtime->buffer_size;
 		if (frames > cont)

@@ -1870,8 +1864,6 @@ static snd_pcm_sframes_t snd_pcm_lib_rea
 
 	if (size == 0)
 		return 0;
-	if (size > runtime->xfer_align)
-		size -= size % runtime->xfer_align;
 
 	snd_pcm_stream_lock_irq(substream);
 	switch (runtime->status->state) {

@@ -1906,12 +1898,12 @@ static snd_pcm_sframes_t snd_pcm_lib_rea
 	      __draining:
 		avail = snd_pcm_capture_avail(runtime);
 		if (runtime->status->state == SNDRV_PCM_STATE_DRAINING) {
-			if (avail < runtime->xfer_align) {
+			if (!avail) {
 				err = -EPIPE;
 				goto _end_unlock;
 			}
-		} else if ((avail < runtime->control->avail_min && size > avail) ||
-			   (size >= runtime->xfer_align && avail < runtime->xfer_align)) {
+		} else if (avail < runtime->control->avail_min &&
+			   size > avail) {
 			wait_queue_t wait;
 			enum { READY, SIGNALED, ERROR, SUSPENDED, EXPIRED, DROPPED } state;
 			long tout;

@@ -1984,8 +1976,6 @@ static snd_pcm_sframes_t snd_pcm_lib_rea
 				break;
 			}
 		}
-		if (avail > runtime->xfer_align)
-			avail -= avail % runtime->xfer_align;
 		frames = size > avail ? avail : size;
 		cont = runtime->buffer_size - runtime->control->appl_ptr % runtime->buffer_size;
 		if (frames > cont)
core/pcm_native.c +0 -13
diff -r 3caa0db34608 core/pcm_native.c
@@ -432,7 +432,6 @@ static int snd_pcm_hw_params(struct snd_
 	runtime->period_step = 1;
 	runtime->sleep_min = 0;
 	runtime->control->avail_min = runtime->period_size;
-	runtime->xfer_align = runtime->period_size;
 	runtime->start_threshold = 1;
 	runtime->stop_threshold = runtime->buffer_size;
 	runtime->silence_threshold = 0;

@@ -529,9 +528,6 @@ static int snd_pcm_sw_params(struct snd_
 		return -EINVAL;
 	if (params->avail_min == 0)
 		return -EINVAL;
-	if (params->xfer_align == 0 ||
-	    params->xfer_align % runtime->min_align != 0)
-		return -EINVAL;
 	if (params->silence_size >= runtime->boundary) {
 		if (params->silence_threshold != 0)
 			return -EINVAL;

@@ -550,7 +546,6 @@ static int snd_pcm_sw_params(struct snd_
 	runtime->stop_threshold = params->stop_threshold;
 	runtime->silence_threshold = params->silence_threshold;
 	runtime->silence_size = params->silence_size;
-	runtime->xfer_align = params->xfer_align;
         params->boundary = runtime->boundary;
 	if (snd_pcm_running(substream)) {
 		if (runtime->sleep_min)

@@ -2282,8 +2277,6 @@ static snd_pcm_sframes_t snd_pcm_playbac
 	}
 	if (frames > (snd_pcm_uframes_t)hw_avail)
 		frames = hw_avail;
-	else
-		frames -= frames % runtime->xfer_align;
 	appl_ptr = runtime->control->appl_ptr - frames;
 	if (appl_ptr < 0)
 		appl_ptr += runtime->boundary;

@@ -2332,8 +2325,6 @@ static snd_pcm_sframes_t snd_pcm_capture
 	}
 	if (frames > (snd_pcm_uframes_t)hw_avail)
 		frames = hw_avail;
-	else
-		frames -= frames % runtime->xfer_align;
 	appl_ptr = runtime->control->appl_ptr - frames;
 	if (appl_ptr < 0)
 		appl_ptr += runtime->boundary;

@@ -2383,8 +2374,6 @@ static snd_pcm_sframes_t snd_pcm_playbac
 	}
 	if (frames > (snd_pcm_uframes_t)avail)
 		frames = avail;
-	else
-		frames -= frames % runtime->xfer_align;
 	appl_ptr = runtime->control->appl_ptr + frames;
 	if (appl_ptr >= (snd_pcm_sframes_t)runtime->boundary)
 		appl_ptr -= runtime->boundary;

@@ -2434,8 +2423,6 @@ static snd_pcm_sframes_t snd_pcm_capture
 	}
 	if (frames > (snd_pcm_uframes_t)avail)
 		frames = avail;
-	else
-		frames -= frames % runtime->xfer_align;
 	appl_ptr = runtime->control->appl_ptr + frames;
 	if (appl_ptr >= (snd_pcm_sframes_t)runtime->boundary)
 		appl_ptr -= runtime->boundary;
include/pcm.h +0 -1
diff -r 3caa0db34608 include/pcm.h
@@ -280,7 +280,6 @@ struct snd_pcm_runtime {
 	int tstamp_mode;		/* mmap timestamp is updated */
   	unsigned int period_step;
 	unsigned int sleep_min;		/* min ticks to sleep */
-	snd_pcm_uframes_t xfer_align;	/* xfer size need to be a multiple */
 	snd_pcm_uframes_t start_threshold;
 	snd_pcm_uframes_t stop_threshold;
 	snd_pcm_uframes_t silence_threshold; /* Silence filling happens when
#5 lore raw

Re: [RFC] Remove PCM xfer_align parameter

Quoted 6 lines · Takashi Iwai

This patch removes the PCM xfer_align parameter. It's still present and won't break the kernel ABI.

Note that this patch won't be applied cleanly to HG, as it's generated in the middle of my patch stack (to fix snd_pcm_lib_write1 blocking bug).

Are you sure that this patch does not change compatibility with older applications? Otherwise, ACK.

Also, alsa-lib API should be updated to respect this (and time tick) changes.

Jaroslav

-----
Jaroslav Kysela <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

author
#6 lore raw

Re: [RFC] Remove PCM xfer_align parameter

At Tue, 8 Jan 2008 17:41:06 +0100 (CET),

Quoted 6 lines · Jaroslav Kysela
Quoted 3 lines · Takashi Iwai

This patch removes the PCM xfer_align parameter. It's still present and won't break the kernel ABI.

Quoted 6 lines
Quoted 3 lines

Note that this patch won't be applied cleanly to HG, as it's generated in the middle of my patch stack (to fix snd_pcm_lib_write1 blocking bug).

Are you sure that this patch does not change compatibility with older applications? Otherwise, ACK.

The xfer_align doesn't actually change the behavior that is exposed to the outside. It's rather the driver internal, and the implementation doesn't influence on the transfer behavior in practice.

Quoted 2 lines

Also, alsa-lib API should be updated to respect this (and time tick) changes.

Yes. That's pending on my local tree.

Also I'll increase PCM pversion too, just to be sure.

Takashi

#7 lore raw

Re: [RFC] Remove PCM xfer_align parameter

Quoted 1 line · Takashi Iwai

Also I'll increase PCM pversion too, just to be sure.

I think that pversion is already incremented because of my monotonic timestamp patch.

Jaroslav

-----
Jaroslav Kysela <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

author
#8 lore raw

Re: [RFC] Remove PCM xfer_align parameter

At Tue, 8 Jan 2008 18:00:36 +0100 (CET),

Quoted 7 lines · Jaroslav Kysela
Quoted 1 line · Takashi Iwai

Also I'll increase PCM pversion too, just to be sure.

I think that pversion is already incremented because of my monotonic timestamp patch.

Ah, then OK, I'll revert that.

thanks,

Takashi

author
#9 lore raw

RFC 1/1 Remove PCM sleep_min and tick

In reply to Takashi Iwai

This patch removes sleep_min and tick stuff from PCM core. It reduces LOC pretty well.

Takashi

core/oss/pcm_oss.c +0 -1
diff -r f325ed8b5f30 core/oss/pcm_oss.c
@@ -985,7 +985,6 @@ static int snd_pcm_oss_change_params(str
 		sw_params->stop_threshold = runtime->buffer_size;
 	sw_params->tstamp_mode = SNDRV_PCM_TSTAMP_NONE;
 	sw_params->period_step = 1;
-	sw_params->sleep_min = 0;
 	sw_params->avail_min = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ?
 		1 : runtime->period_size;
 	if (atomic_read(&substream->mmap_count) ||
core/pcm.c +0 -11
diff -r f325ed8b5f30 core/pcm.c
@@ -359,7 +359,6 @@ static void snd_pcm_substream_proc_hw_pa
 	snd_iprintf(buffer, "rate: %u (%u/%u)\n", runtime->rate, runtime->rate_num, runtime->rate_den);	
 	snd_iprintf(buffer, "period_size: %lu\n", runtime->period_size);	
 	snd_iprintf(buffer, "buffer_size: %lu\n", runtime->buffer_size);	
-	snd_iprintf(buffer, "tick_time: %u\n", runtime->tick_time);
 #if defined(CONFIG_SND_PCM_OSS) || defined(CONFIG_SND_PCM_OSS_MODULE)
 	if (substream->oss.oss) {
 		snd_iprintf(buffer, "OSS format: %s\n", snd_pcm_oss_format_name(runtime->oss.format));

@@ -387,7 +386,6 @@ static void snd_pcm_substream_proc_sw_pa
 	}
 	snd_iprintf(buffer, "tstamp_mode: %s\n", snd_pcm_tstamp_mode_name(runtime->tstamp_mode));
 	snd_iprintf(buffer, "period_step: %u\n", runtime->period_step);
-	snd_iprintf(buffer, "sleep_min: %u\n", runtime->sleep_min);
 	snd_iprintf(buffer, "avail_min: %lu\n", runtime->control->avail_min);
 	snd_iprintf(buffer, "start_threshold: %lu\n", runtime->start_threshold);
 	snd_iprintf(buffer, "stop_threshold: %lu\n", runtime->stop_threshold);

@@ -764,12 +762,6 @@ static int snd_pcm_dev_free(struct snd_d
 	return snd_pcm_free(pcm);
 }
 
-static void snd_pcm_tick_timer_func(unsigned long data)
-{
-	struct snd_pcm_substream *substream = (struct snd_pcm_substream *) data;
-	snd_pcm_tick_elapsed(substream);
-}
-
 int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
 			     struct file *file,
 			     struct snd_pcm_substream **rsubstream)

@@ -876,9 +868,6 @@ int snd_pcm_attach_substream(struct snd_
 	memset((void*)runtime->control, 0, size);
 
 	init_waitqueue_head(&runtime->sleep);
-	init_timer(&runtime->tick_timer);
-	runtime->tick_timer.function = snd_pcm_tick_timer_func;
-	runtime->tick_timer.data = (unsigned long) substream;
 
 	runtime->status->state = SNDRV_PCM_STATE_OPEN;
core/pcm_lib.c +3 -110

155 lines not on this page · raw message

core/pcm_native.c +0 -30
diff -r f325ed8b5f30 core/pcm_native.c
@@ -410,7 +410,6 @@ static int snd_pcm_hw_params(struct snd_
 	runtime->period_size = params_period_size(params);
 	runtime->periods = params_periods(params);
 	runtime->buffer_size = params_buffer_size(params);
-	runtime->tick_time = params_tick_time(params);
 	runtime->info = params->info;
 	runtime->rate_num = params->rate_num;
 	runtime->rate_den = params->rate_den;

@@ -430,7 +429,6 @@ static int snd_pcm_hw_params(struct snd_
 	/* Default sw params */
 	runtime->tstamp_mode = SNDRV_PCM_TSTAMP_NONE;
 	runtime->period_step = 1;
-	runtime->sleep_min = 0;
 	runtime->control->avail_min = runtime->period_size;
 	runtime->start_threshold = 1;
 	runtime->stop_threshold = runtime->buffer_size;

@@ -539,7 +537,6 @@ static int snd_pcm_sw_params(struct snd_
 	}
 	snd_pcm_stream_lock_irq(substream);
 	runtime->tstamp_mode = params->tstamp_mode;
-	runtime->sleep_min = params->sleep_min;
 	runtime->period_step = params->period_step;
 	runtime->control->avail_min = params->avail_min;
 	runtime->start_threshold = params->start_threshold;

@@ -548,10 +545,6 @@ static int snd_pcm_sw_params(struct snd_
 	runtime->silence_size = params->silence_size;
         params->boundary = runtime->boundary;
 	if (snd_pcm_running(substream)) {
-		if (runtime->sleep_min)
-			snd_pcm_tick_prepare(substream);
-		else
-			snd_pcm_tick_set(substream, 0);
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
 		    runtime->silence_size > 0)
 			snd_pcm_playback_silence(substream, ULONG_MAX);

@@ -901,8 +894,6 @@ static void snd_pcm_post_start(struct sn
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
 	    runtime->silence_size > 0)
 		snd_pcm_playback_silence(substream, ULONG_MAX);
-	if (runtime->sleep_min)
-		snd_pcm_tick_prepare(substream);
 	if (substream->timer)
 		snd_timer_notify(substream->timer, SNDRV_TIMER_EVENT_MSTART,
 				 &runtime->trigger_tstamp);

@@ -956,7 +947,6 @@ static void snd_pcm_post_stop(struct snd
 			snd_timer_notify(substream->timer, SNDRV_TIMER_EVENT_MSTOP,
 					 &runtime->trigger_tstamp);
 		runtime->status->state = state;
-		snd_pcm_tick_set(substream, 0);
 	}
 	wake_up(&runtime->sleep);
 }

@@ -1040,12 +1030,9 @@ static void snd_pcm_post_pause(struct sn
 			snd_timer_notify(substream->timer,
 					 SNDRV_TIMER_EVENT_MPAUSE,
 					 &runtime->trigger_tstamp);
-		snd_pcm_tick_set(substream, 0);
 		wake_up(&runtime->sleep);
 	} else {
 		runtime->status->state = SNDRV_PCM_STATE_RUNNING;
-		if (runtime->sleep_min)
-			snd_pcm_tick_prepare(substream);
 		if (substream->timer)
 			snd_timer_notify(substream->timer,
 					 SNDRV_TIMER_EVENT_MCONTINUE,

@@ -1100,7 +1087,6 @@ static void snd_pcm_post_suspend(struct 
 				 &runtime->trigger_tstamp);
 	runtime->status->suspended_state = runtime->status->state;
 	runtime->status->state = SNDRV_PCM_STATE_SUSPENDED;
-	snd_pcm_tick_set(substream, 0);
 	wake_up(&runtime->sleep);
 }
 

@@ -1203,8 +1189,6 @@ static void snd_pcm_post_resume(struct s
 		snd_timer_notify(substream->timer, SNDRV_TIMER_EVENT_MRESUME,
 				 &runtime->trigger_tstamp);
 	runtime->status->state = runtime->status->suspended_state;
-	if (runtime->sleep_min)
-		snd_pcm_tick_prepare(substream);
 }
 
 static struct action_ops snd_pcm_action_resume = {

@@ -2040,8 +2024,6 @@ int snd_pcm_hw_constraints_complete(stru
 	}
 
 	/* FIXME: this belong to lowlevel */
-	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_TICK_TIME,
-				     1000000 / HZ, 1000000 / HZ);
 	snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE);
 
 	return 0;

@@ -2281,9 +2263,6 @@ static snd_pcm_sframes_t snd_pcm_playbac
 	if (appl_ptr < 0)
 		appl_ptr += runtime->boundary;
 	runtime->control->appl_ptr = appl_ptr;
-	if (runtime->status->state == SNDRV_PCM_STATE_RUNNING &&
-	    runtime->sleep_min)
-		snd_pcm_tick_prepare(substream);
 	ret = frames;
  __end:
 	snd_pcm_stream_unlock_irq(substream);

@@ -2329,9 +2308,6 @@ static snd_pcm_sframes_t snd_pcm_capture
 	if (appl_ptr < 0)
 		appl_ptr += runtime->boundary;
 	runtime->control->appl_ptr = appl_ptr;
-	if (runtime->status->state == SNDRV_PCM_STATE_RUNNING &&
-	    runtime->sleep_min)
-		snd_pcm_tick_prepare(substream);
 	ret = frames;
  __end:
 	snd_pcm_stream_unlock_irq(substream);

@@ -2378,9 +2354,6 @@ static snd_pcm_sframes_t snd_pcm_playbac
 	if (appl_ptr >= (snd_pcm_sframes_t)runtime->boundary)
 		appl_ptr -= runtime->boundary;
 	runtime->control->appl_ptr = appl_ptr;
-	if (runtime->status->state == SNDRV_PCM_STATE_RUNNING &&
-	    runtime->sleep_min)
-		snd_pcm_tick_prepare(substream);
 	ret = frames;
  __end:
 	snd_pcm_stream_unlock_irq(substream);

@@ -2427,9 +2400,6 @@ static snd_pcm_sframes_t snd_pcm_capture
 	if (appl_ptr >= (snd_pcm_sframes_t)runtime->boundary)
 		appl_ptr -= runtime->boundary;
 	runtime->control->appl_ptr = appl_ptr;
-	if (runtime->status->state == SNDRV_PCM_STATE_RUNNING &&
-	    runtime->sleep_min)
-		snd_pcm_tick_prepare(substream);
 	ret = frames;
  __end:
 	snd_pcm_stream_unlock_irq(substream);
include/pcm.h +0 -7
diff -r f325ed8b5f30 include/pcm.h
@@ -267,7 +267,6 @@ struct snd_pcm_runtime {
 	snd_pcm_uframes_t period_size;	/* period size */
 	unsigned int periods;		/* periods */
 	snd_pcm_uframes_t buffer_size;	/* buffer size */
-	unsigned int tick_time;		/* tick time */
 	snd_pcm_uframes_t min_align;	/* Min alignment for the format */
 	size_t byte_align;
 	unsigned int frame_bits;

@@ -279,7 +278,6 @@ struct snd_pcm_runtime {
 	/* -- SW params -- */
 	int tstamp_mode;		/* mmap timestamp is updated */
   	unsigned int period_step;
-	unsigned int sleep_min;		/* min ticks to sleep */
 	snd_pcm_uframes_t start_threshold;
 	snd_pcm_uframes_t stop_threshold;
 	snd_pcm_uframes_t silence_threshold; /* Silence filling happens when

@@ -298,7 +296,6 @@ struct snd_pcm_runtime {
 
 	/* -- locking / scheduling -- */
 	wait_queue_head_t sleep;
-	struct timer_list tick_timer;
 	struct fasync_struct *fasync;
 
 	/* -- private section -- */

@@ -803,7 +800,6 @@ static inline const struct snd_interval 
 #define params_periods(p) hw_param_interval((p), SNDRV_PCM_HW_PARAM_PERIODS)->min
 #define params_buffer_size(p) hw_param_interval((p), SNDRV_PCM_HW_PARAM_BUFFER_SIZE)->min
 #define params_buffer_bytes(p) hw_param_interval((p), SNDRV_PCM_HW_PARAM_BUFFER_BYTES)->min
-#define params_tick_time(p) hw_param_interval((p), SNDRV_PCM_HW_PARAM_TICK_TIME)->min
 
 
 int snd_interval_refine(struct snd_interval *i, const struct snd_interval *v);

@@ -901,9 +897,6 @@ int snd_pcm_playback_xrun_asap(struct sn
 int snd_pcm_playback_xrun_asap(struct snd_pcm_substream *substream);
 int snd_pcm_capture_xrun_asap(struct snd_pcm_substream *substream);
 void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_uframes_t new_hw_ptr);
-void snd_pcm_tick_prepare(struct snd_pcm_substream *substream);
-void snd_pcm_tick_set(struct snd_pcm_substream *substream, unsigned long ticks);
-void snd_pcm_tick_elapsed(struct snd_pcm_substream *substream);
 void snd_pcm_period_elapsed(struct snd_pcm_substream *substream);
 snd_pcm_sframes_t snd_pcm_lib_write(struct snd_pcm_substream *substream,
 				    const void __user *buf,
#10 lore raw

Re: [RFC] Remove PCM sleep_min and tick

Quoted 2 lines · Takashi Iwai

This patch removes sleep_min and tick stuff from PCM core. It reduces LOC pretty well.

Ack.

Jaroslav

-----
Jaroslav Kysela <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

author
#11 lore raw

RFC 1/1 Remove sound/driver.h

In reply to Takashi Iwai

This is no concrete patch, but just shows you an essence.

In many places, simply the inclusion of sound/driver.h is removed:

isa/als100.c +0 -1
diff -r 1f5b3ddb6636 isa/als100.c
@@ -20,7 +20,6 @@
     Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
 */
 
-#include <sound/driver.h>
 #include <linux/init.h>
 #include <linux/wait.h>
 #include <linux/time.h>

And, core.h and driver.h are changed like below.

include/core.h +10 -0
diff -r 1f5b3ddb6636 include/core.h
@@ -22,11 +22,21 @@
  *
  */
 
+#include <linux/module.h>
 #include <linux/sched.h>		/* wake_up() */
 #include <linux/mutex.h>		/* struct mutex */
 #include <linux/rwsem.h>		/* struct rw_semaphore */
 #include <linux/pm.h>			/* pm_message_t */
 #include <linux/device.h>
+
+/* number of supported soundcards */
+#ifdef CONFIG_SND_DYNAMIC_MINORS
+#define SNDRV_CARDS 32
+#else
+#define SNDRV_CARDS 8		/* don't change - minor numbers */
+#endif
+
+#define CONFIG_SND_MAJOR	116	/* standard configuration */
 
 /* forward declarations */
 #ifdef CONFIG_PCI
include/driver.h +1 -47

48 lines not on this page · raw message

The whole patch is too big to post here.

Takashi

#12 lore raw

Re: [RFC] Remove sound/driver.h

isa/als100.c line 23

Quoted 12 lines · Takashi Iwai

This is no concrete patch, but just shows you an essence.

In many places, simply the inclusion of sound/driver.h is removed:

diff -r 1f5b3ddb6636 isa/als100.c

isa/als100.c +0 -1
@@ -20,7 +20,6 @@
     Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
 */
 
-#include <sound/driver.h>

Ack. How you would like to solve the alsa-driver internal build?

Jaroslav

-----
Jaroslav Kysela <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

author
#13 lore raw

Re: [RFC] Remove sound/driver.h

At Tue, 8 Jan 2008 17:37:21 +0100 (CET),

Quoted 14 lines · Jaroslav Kysela
Quoted 11 lines · Takashi Iwai

This is no concrete patch, but just shows you an essence.

In many places, simply the inclusion of sound/driver.h is removed:

diff -r 1f5b3ddb6636 isa/als100.c

isa/als100.c +0 -0
@@ -20,7 +20,6 @@
     Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
 */
Quoted 3 lines

isa/als100.c line 23

Quoted 1 line
isa/als100.c +0 -1
-#include <sound/driver.h>

Ack. How you would like to solve the alsa-driver internal build?

By simply adding '#include "adriver.h" in each *.c in alsa-driver tree.

Takashi

#14 lore raw

Re: [RFC] Remove sound/driver.h

Quoted 16 lines · Takashi Iwai

At Tue, 8 Jan 2008 17:37:21 +0100 (CET),

Quoted 14 lines · Jaroslav Kysela
Quoted 11 lines · Takashi Iwai

This is no concrete patch, but just shows you an essence.

In many places, simply the inclusion of sound/driver.h is removed:

diff -r 1f5b3ddb6636 isa/als100.c

isa/als100.c +0 -0
@@ -20,7 +20,6 @@
     Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
 */
Quoted 6 lines
Quoted 3 lines

isa/als100.c line 23

Quoted 1 line
isa/als100.c +0 -1
-#include <sound/driver.h>

Ack. How you would like to solve the alsa-driver internal build?

By simply adding '#include "adriver.h" in each *.c in alsa-driver tree.

Good. Thanks.

Jaroslav

-----
Jaroslav Kysela <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

#15 lore raw
In reply to Takashi Iwai

Please Do!

Quoted 60 lines · On Thu, 20 Dec 2007 15:55:56 +0100 "Takashi Iwai" <[email protected]> wrote:

Hi,

ALSA kernel tree has lots of unused and rotten codes. I'd like to clean up this. Here we go...

1. Indirect control element access

This is what I already suggested. This hasn't been used (we have no proper API in alsa-lib), and 32/64bit wrapper doesn't work at all.

Also, the idea to copy such a big data area via ioctl is bad. If we need a big matrix mixer, let make each matrix element accessible individually. Copying the whole matrix at each time isn't efficient.

2. PCM xfer_align parameter

This sw_params parameter has never been used in a sane manner, and no one understands what this does exactly. The current implementation looks also buggy because it allows write of shorter size than xfer_align. So, if you do partial writes, the write isn't actually aligned at all.

Removing this parameter will make some pcm_lib_* code more readable (and less buggy).

3. PCM sleep_min and tick

The "tick" in PCM is set (again) via sw_params. And, nobody uses this feature at all except for a command line option of aplay. (This is literally "nobody", as I checked alsa-lib API calls in all programs in some major distros.)

Above all, if we need finer wake-ups for the position update, it's basically an issue that the driver should solve, not tuned by each application.

4. sound/driver.h

This header file exists only for some hacks to adapt alsa-driver tree. It's useless for building in the kernel. Let's move a few lines in it to sound/core.h and remove it.

For building the modules on alsa-driver external tree, we can simply add

    #include "adriver.h"

to each build stub (alsa-driver/*/*.c) before inclusion of alsa-kernel codes.

I'll post some patches to clean the above thing up in the following posts. Please speak up if you have objections.

Takashi
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

#16 lore raw
In reply to Takashi Iwai
Quoted 5 lines · Takashi Iwai

Hi,

ALSA kernel tree has lots of unused and rotten codes. I'd like to clean up this. Here we go...

Quoted 7 lines

1. Indirect control element access
2. PCM xfer_align parameter
3. PCM sleep_min and tick
4. sound/driver.h

I'll post some patches to clean the above thing up in the following posts. Please speak up if you have objections.

Please wait to first week in 2008. I'll review your patches. But preliminary, I agree. If code is not used, it's better to remove it.

Jaroslav

-----
Jaroslav Kysela <[email protected]>
Linux Kernel Sound Maintainer
ALSA Project