ALSA: Get rid of card power_lock
authorTakashi Iwai <tiwai@suse.de>
Wed, 30 Aug 2017 14:13:25 +0000 (16:13 +0200)
committerTakashi Iwai <tiwai@suse.de>
Wed, 30 Aug 2017 18:44:29 +0000 (20:44 +0200)
Currently we're taking power_lock at each card component for assuring
the power-up sequence, but it doesn't help anything in the
implementation at the moment: it just serializes unnecessarily the
callers, but it doesn't protect about the power state change itself.
It used to have some usefulness in the early days where we managed the
PM manually.  But now the suspend/resume core procedure is beyond our
hands, and power_lock lost its meaning.

This patch drops the power_lock from allover the places.
There shouldn't be any issues by this change, as it's no helper
regarding the power state change.  Rather we'll get better performance
by removing the serialization; which is the only slight concern of any
behavior change, but it can't be a showstopper, after all.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
include/sound/core.h
sound/core/control.c
sound/core/control_compat.c
sound/core/init.c
sound/core/pcm_native.c
sound/soc/soc-core.c

index 357f36b5ee80403edff7f39947a36aeda022c6ae..4104a9d1001fcb0f14709c0ad029561e6b03eefe 100644 (file)
@@ -136,7 +136,6 @@ struct snd_card {
 
 #ifdef CONFIG_PM
        unsigned int power_state;       /* power state */
-       struct mutex power_lock;        /* power lock */
        wait_queue_head_t power_sleep;
 #endif
 
@@ -149,16 +148,6 @@ struct snd_card {
 #define dev_to_snd_card(p)     container_of(p, struct snd_card, card_dev)
 
 #ifdef CONFIG_PM
-static inline void snd_power_lock(struct snd_card *card)
-{
-       mutex_lock(&card->power_lock);
-}
-
-static inline void snd_power_unlock(struct snd_card *card)
-{
-       mutex_unlock(&card->power_lock);
-}
-
 static inline unsigned int snd_power_get_state(struct snd_card *card)
 {
        return card->power_state;
@@ -175,8 +164,6 @@ int snd_power_wait(struct snd_card *card, unsigned int power_state);
 
 #else /* ! CONFIG_PM */
 
-#define snd_power_lock(card)           do { (void)(card); } while (0)
-#define snd_power_unlock(card)         do { (void)(card); } while (0)
 static inline int snd_power_wait(struct snd_card *card, unsigned int state) { return 0; }
 #define snd_power_get_state(card)      ({ (void)(card); SNDRV_CTL_POWER_D0; })
 #define snd_power_change_state(card, state)    do { (void)(card); } while (0)
index 51d4b4ad3e1d93d982b9b956793d4eeb9c159f19..56b3e2d49c82321509e699217ee1085382c40ce4 100644 (file)
@@ -864,14 +864,14 @@ static int snd_ctl_elem_info_user(struct snd_ctl_file *ctl,
 
        if (copy_from_user(&info, _info, sizeof(info)))
                return -EFAULT;
-       snd_power_lock(ctl->card);
        result = snd_power_wait(ctl->card, SNDRV_CTL_POWER_D0);
-       if (result >= 0)
-               result = snd_ctl_elem_info(ctl, &info);
-       snd_power_unlock(ctl->card);
-       if (result >= 0)
-               if (copy_to_user(_info, &info, sizeof(info)))
-                       return -EFAULT;
+       if (result < 0)
+               return result;
+       result = snd_ctl_elem_info(ctl, &info);
+       if (result < 0)
+               return result;
+       if (copy_to_user(_info, &info, sizeof(info)))
+               return -EFAULT;
        return result;
 }
 
@@ -905,17 +905,19 @@ static int snd_ctl_elem_read_user(struct snd_card *card,
        if (IS_ERR(control))
                return PTR_ERR(control);
 
-       snd_power_lock(card);
        result = snd_power_wait(card, SNDRV_CTL_POWER_D0);
-       if (result >= 0) {
-               down_read(&card->controls_rwsem);
-               result = snd_ctl_elem_read(card, control);
-               up_read(&card->controls_rwsem);
-       }
-       snd_power_unlock(card);
-       if (result >= 0)
-               if (copy_to_user(_control, control, sizeof(*control)))
-                       result = -EFAULT;
+       if (result < 0)
+               goto error;
+
+       down_read(&card->controls_rwsem);
+       result = snd_ctl_elem_read(card, control);
+       up_read(&card->controls_rwsem);
+       if (result < 0)
+               goto error;
+
+       if (copy_to_user(_control, control, sizeof(*control)))
+               result = -EFAULT;
+ error:
        kfree(control);
        return result;
 }
@@ -964,17 +966,19 @@ static int snd_ctl_elem_write_user(struct snd_ctl_file *file,
                return PTR_ERR(control);
 
        card = file->card;
-       snd_power_lock(card);
        result = snd_power_wait(card, SNDRV_CTL_POWER_D0);
-       if (result >= 0) {
-               down_write(&card->controls_rwsem);
-               result = snd_ctl_elem_write(card, file, control);
-               up_write(&card->controls_rwsem);
-       }
-       snd_power_unlock(card);
-       if (result >= 0)
-               if (copy_to_user(_control, control, sizeof(*control)))
-                       result = -EFAULT;
+       if (result < 0)
+               goto error;
+
+       down_write(&card->controls_rwsem);
+       result = snd_ctl_elem_write(card, file, control);
+       up_write(&card->controls_rwsem);
+       if (result < 0)
+               goto error;
+
+       if (copy_to_user(_control, control, sizeof(*control)))
+               result = -EFAULT;
+ error:
        kfree(control);
        return result;
 }
index 1fa70766ffabedea3cd5d3bf20779dfb7e3fcff8..a848836a5de0468534d5eecfd24adf4bc743f9f2 100644 (file)
@@ -111,12 +111,10 @@ static int snd_ctl_elem_info_compat(struct snd_ctl_file *ctl,
        if (get_user(data->value.enumerated.item, &data32->value.enumerated.item))
                goto error;
 
-       snd_power_lock(ctl->card);
        err = snd_power_wait(ctl->card, SNDRV_CTL_POWER_D0);
-       if (err >= 0)
-               err = snd_ctl_elem_info(ctl, data);
-       snd_power_unlock(ctl->card);
-
+       if (err < 0)
+               goto error;
+       err = snd_ctl_elem_info(ctl, data);
        if (err < 0)
                goto error;
        /* restore info to 32bit */
@@ -315,14 +313,13 @@ static int ctl_elem_read_user(struct snd_card *card,
        if (err < 0)
                goto error;
 
-       snd_power_lock(card);
        err = snd_power_wait(card, SNDRV_CTL_POWER_D0);
-       if (err >= 0)
-               err = snd_ctl_elem_read(card, data);
-       snd_power_unlock(card);
-       if (err >= 0)
-               err = copy_ctl_value_to_user(userdata, valuep, data,
-                                            type, count);
+       if (err < 0)
+               goto error;
+       err = snd_ctl_elem_read(card, data);
+       if (err < 0)
+               goto error;
+       err = copy_ctl_value_to_user(userdata, valuep, data, type, count);
  error:
        kfree(data);
        return err;
@@ -344,14 +341,13 @@ static int ctl_elem_write_user(struct snd_ctl_file *file,
        if (err < 0)
                goto error;
 
-       snd_power_lock(card);
        err = snd_power_wait(card, SNDRV_CTL_POWER_D0);
-       if (err >= 0)
-               err = snd_ctl_elem_write(card, file, data);
-       snd_power_unlock(card);
-       if (err >= 0)
-               err = copy_ctl_value_to_user(userdata, valuep, data,
-                                            type, count);
+       if (err < 0)
+               goto error;
+       err = snd_ctl_elem_write(card, file, data);
+       if (err < 0)
+               goto error;
+       err = copy_ctl_value_to_user(userdata, valuep, data, type, count);
  error:
        kfree(data);
        return err;
index 6e219dc23f9653547927f0fa0956a4cc581ae379..32ebe2f6bc59667c63624710ecd16695e96d42b5 100644 (file)
@@ -253,7 +253,6 @@ int snd_card_new(struct device *parent, int idx, const char *xid,
        spin_lock_init(&card->files_lock);
        INIT_LIST_HEAD(&card->files_list);
 #ifdef CONFIG_PM
-       mutex_init(&card->power_lock);
        init_waitqueue_head(&card->power_sleep);
 #endif
 
@@ -978,8 +977,6 @@ EXPORT_SYMBOL(snd_card_file_remove);
  *  Waits until the power-state is changed.
  *
  *  Return: Zero if successful, or a negative error code.
- *
- *  Note: the power lock must be active before call.
  */
 int snd_power_wait(struct snd_card *card, unsigned int power_state)
 {
@@ -999,9 +996,7 @@ int snd_power_wait(struct snd_card *card, unsigned int power_state)
                if (snd_power_get_state(card) == power_state)
                        break;
                set_current_state(TASK_UNINTERRUPTIBLE);
-               snd_power_unlock(card);
                schedule_timeout(30 * HZ);
-               snd_power_lock(card);
        }
        remove_wait_queue(&card->power_sleep, &wait);
        return result;
index cf0433f8006772392061b0cf95514a266a44f948..621142ea9ec63d545b1ec0b0ffe8c6624817d4b9 100644 (file)
@@ -1830,7 +1830,6 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
                add_wait_queue(&to_check->sleep, &wait);
                snd_pcm_stream_unlock_irq(substream);
                up_read(&snd_pcm_link_rwsem);
-               snd_power_unlock(card);
                if (runtime->no_period_wakeup)
                        tout = MAX_SCHEDULE_TIMEOUT;
                else {
@@ -1842,7 +1841,6 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream,
                        tout = msecs_to_jiffies(tout * 1000);
                }
                tout = schedule_timeout_interruptible(tout);
-               snd_power_lock(card);
                down_read(&snd_pcm_link_rwsem);
                snd_pcm_stream_lock_irq(substream);
                remove_wait_queue(&to_check->sleep, &wait);
@@ -2764,11 +2762,16 @@ static int snd_pcm_tstamp(struct snd_pcm_substream *substream, int __user *_arg)
        return 0;
 }
                
-static int snd_pcm_common_ioctl(struct file *file,
+static int snd_pcm_common_ioctl1(struct file *file,
                                 struct snd_pcm_substream *substream,
                                 unsigned int cmd, void __user *arg)
 {
        struct snd_pcm_file *pcm_file = file->private_data;
+       int res;
+
+       res = snd_power_wait(substream->pcm->card, SNDRV_CTL_POWER_D0);
+       if (res < 0)
+               return res;
 
        switch (cmd) {
        case SNDRV_PCM_IOCTL_PVERSION:
@@ -2846,21 +2849,6 @@ static int snd_pcm_common_ioctl(struct file *file,
        return -ENOTTY;
 }
 
-static int snd_pcm_common_ioctl1(struct file *file,
-                                struct snd_pcm_substream *substream,
-                                unsigned int cmd, void __user *arg)
-{
-       struct snd_card *card = substream->pcm->card;
-       int res;
-
-       snd_power_lock(card);
-       res = snd_power_wait(card, SNDRV_CTL_POWER_D0);
-       if (res >= 0)
-               res = snd_pcm_common_ioctl(file, substream, cmd, arg);
-       snd_power_unlock(card);
-       return res;
-}
-
 static int snd_pcm_playback_ioctl1(struct file *file,
                                   struct snd_pcm_substream *substream,
                                   unsigned int cmd, void __user *arg)
@@ -3064,7 +3052,6 @@ int snd_pcm_kernel_ioctl(struct snd_pcm_substream *substream,
 {
        snd_pcm_uframes_t *frames = arg;
        snd_pcm_sframes_t result;
-       int err;
        
        switch (cmd) {
        case SNDRV_PCM_IOCTL_FORWARD:
@@ -3084,10 +3071,7 @@ int snd_pcm_kernel_ioctl(struct snd_pcm_substream *substream,
        case SNDRV_PCM_IOCTL_START:
                return snd_pcm_start_lock_irq(substream);
        case SNDRV_PCM_IOCTL_DRAIN:
-               snd_power_lock(substream->pcm->card);
-               err = snd_pcm_drain(substream, NULL);
-               snd_power_unlock(substream->pcm->card);
-               return err;
+               return snd_pcm_drain(substream, NULL);
        case SNDRV_PCM_IOCTL_DROP:
                return snd_pcm_drop(substream);
        case SNDRV_PCM_IOCTL_DELAY:
index 13c875e2392a40ec5651d7c12a28b9ac9f3aab85..62c11e26ce5cc8cbbfe89957f73a1ed0cd65b0f9 100644 (file)
@@ -653,9 +653,7 @@ int snd_soc_suspend(struct device *dev)
        /* Due to the resume being scheduled into a workqueue we could
        * suspend before that's finished - wait for it to complete.
         */
-       snd_power_lock(card->snd_card);
        snd_power_wait(card->snd_card, SNDRV_CTL_POWER_D0);
-       snd_power_unlock(card->snd_card);
 
        /* we're going to block userspace touching us until resume completes */
        snd_power_change_state(card->snd_card, SNDRV_CTL_POWER_D3hot);