Received-SPF: pass (google.com: domain of linux-kernel+bounces-102415-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <d97f635f-053b-70a7-5ffe-a1ae273091d1@quicinc.com>
Date: Wed, 13 Mar 2024 12:18:45 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [PATCH v18 20/41] ALSA: usb-audio: qcom: Introduce QC USB SND
 offloading support
Content-Language: en-US
To: Albert Wang <albertccwang@google.com>
CC: <srinivas.kandagatla@linaro.org>, <mathias.nyman@intel.com>,
        <perex@perex.cz>, <conor+dt@kernel.org>, <corbet@lwn.net>,
        <lgirdwood@gmail.com>, <andersson@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <gregkh@linuxfoundation.org>,
        <Thinh.Nguyen@synopsys.com>, <broonie@kernel.org>,
        <bgoswami@quicinc.com>, <tiwai@suse.com>, <robh+dt@kernel.org>,
        <konrad.dybcio@linaro.org>, <linux-kernel@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-sound@vger.kernel.org>,
        <linux-usb@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-doc@vger.kernel.org>, <alsa-devel@alsa-project.org>
References: <20240228013619.29758-1-quic_wcheng@quicinc.com>
 <20240228013619.29758-21-quic_wcheng@quicinc.com>
 <CANqn-rjTgHgzssxZiuwvTKzOS31wzjS4Y9G-XacZN4a7c82MaA@mail.gmail.com>
From: Wesley Cheng <quic_wcheng@quicinc.com>
In-Reply-To: <CANqn-rjTgHgzssxZiuwvTKzOS31wzjS4Y9G-XacZN4a7c82MaA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit

Hi Albert,

On 3/13/2024 1:03 AM, Albert Wang wrote:
> +/**
> + * qc_usb_audio_offload_suspend() - USB offload PM suspend handler
> + * @intf: USB interface
> + * @message: suspend type
> + *
> + * PM suspend handler to ensure that the USB offloading driver is able to stop
> + * any pending traffic, so that the bus can be suspended.
> + *
> + */
> +static void qc_usb_audio_offload_suspend(struct usb_interface *intf,
> +                                               pm_message_t message)
> +{
> +       struct qmi_uaudio_stream_ind_msg_v01 disconnect_ind = {0};
> +       struct snd_usb_audio *chip = usb_get_intfdata(intf);
> +       struct uaudio_qmi_svc *svc = uaudio_svc;
> +       struct uaudio_dev *dev;
> +       int card_num;
> +       int ret;
> +
> +       if (!chip)
> +               return;
> +
> +       card_num = chip->card->number;
> +       if (card_num >= SNDRV_CARDS)
> +               return;
> +
> +
> +       mutex_lock(&chip->mutex);
> +       dev = &uadev[card_num];
> +
> +       if (atomic_read(&dev->in_use)) {
> +               mutex_unlock(&chip->mutex);
> +               dev_dbg(uaudio_qdev->data->dev, "sending qmi
> indication suspend\n");
> +               disconnect_ind.dev_event = USB_QMI_DEV_DISCONNECT_V01;
> +               disconnect_ind.slot_id = dev->udev->slot_id;
> +               disconnect_ind.controller_num = dev->usb_core_id;
> +               disconnect_ind.controller_num_valid = 1;
> +               ret = qmi_send_indication(svc->uaudio_svc_hdl, &svc->client_sq,
> +                               QMI_UAUDIO_STREAM_IND_V01,
> +                               QMI_UAUDIO_STREAM_IND_MSG_V01_MAX_MSG_LEN,
> +                               qmi_uaudio_stream_ind_msg_v01_ei,
> +                               &disconnect_ind);
> +               if (ret < 0)
> +                       dev_err(uaudio_qdev->data->dev,
> +                               "qmi send failed with err: %d\n", ret);
> +
> +               ret = wait_event_interruptible_timeout(dev->disconnect_wq,
> +                               !atomic_read(&dev->in_use),
> +                               msecs_to_jiffies(DEV_RELEASE_WAIT_TIMEOUT));
> +               if (!ret) {
> +                       dev_err(uaudio_qdev->data->dev,
> +                               "timeout while waiting for dev_release\n");
> +                       atomic_set(&dev->in_use, 0);
> +               } else if (ret < 0) {
> +                       dev_err(uaudio_qdev->data->dev,
> +                               "failed with ret %d\n", ret);
> +                               atomic_set(&dev->in_use, 0);
> +               }
> +               mutex_lock(&chip->mutex);
> +       }
> +       mutex_unlock(&chip->mutex);
> +}
> +
> 
> Hi Wesley,
> 
> The suspend function `qc_usb_audio_offload_suspend()` looks to stop
> the traffic on the bus, so that the bus can be suspended. That allows
> the application processor(AP) to enter suspend. There is a subtle
> difference with our feature, which is to allow AP suspend with the
> Host and USB controller active to continue the audio offloading. We
> call this feature `allow AP suspend in playback`. So, I have some
> points to clarify with you:

Yes, I'm aware of that feature also.

> 1. Will the suspend flow `usb_audio_suspend() -->
> platform_ops->suspend_cb() --> qc_usb_audio_offload_suspend()` be
> called when offloading is active?

It can be.  This is why in our case, we are going to issue the 
disconnect event to the audio DSP to stop the session if it is currently 
in one.

> 2. As my understanding, the suspend function is to allow AP suspend
> when the offloading is IDLE, but it won't allow AP suspend when in
> playback or capture. Please correct me if anything is wrong.

As mentioned above, it will let apps go into PM suspend after forcing 
the audio stream to be idle.  We won't block PM suspend entry.

> 3. We would like to integrate the `allow AP suspend in playback`
> feature with your framework to become one upstream offload solution.
> Here is the patch:
> https://patchwork.kernel.org/project/linux-pm/patch/20240223143833.1509961-1-guanyulin@google.com/
> .

Yes, I saw that patch as well.  I'll take a look once this series lands 
upstream.

Thanks
Wesley Cheng