Re: [PATCH] bus: mhi: host: pci: Disable runtime PM for QDU100

[Vivek Pernamitta <quic_vpernami@quicinc.com>]

On 4/18/2025 2:06 PM, Manivannan Sadhasivam wrote:
> On Fri, Apr 18, 2025 at 11:55:24AM +0530, Vivek Pernamitta wrote:
>>
>>
>> On 4/17/2025 11:37 AM, Manivannan Sadhasivam wrote:
>>> On Thu, Apr 17, 2025 at 10:00:38AM +0530, Veerabhadrarao Badiganti wrote:
>>>>
>>>> On 4/14/2025 1:17 PM, Vivek Pernamitta wrote:
>>>>> The QDU100 device does not support the MHI M3 state, necessitating the
>>>>> disabling of runtime PM for this device. Since the PCIe core framework
>>>>> enables runtime PM by default for all clients, it is essential to disable
>>>>> runtime PM if the device does not support Low Power Mode (LPM).
>>>>>
> 
> Not true... See below.
> 
>>>>> Signed-off-by: Vivek Pernamitta<quic_vpernami@quicinc.com>
>>>>> ---
>>>>>     drivers/bus/mhi/host/pci_generic.c | 10 ++++++++++
>>>>>     1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>>>>> index 03aa887952098661a488650053a357f883d1559b..a011fd2d48c57cf9d1aec74040153267a206d797 100644
>>>>> --- a/drivers/bus/mhi/host/pci_generic.c
>>>>> +++ b/drivers/bus/mhi/host/pci_generic.c
>>>>> @@ -43,6 +43,7 @@
>>>>>      * @mru_default: default MRU size for MBIM network packets
>>>>>      * @sideband_wake: Devices using dedicated sideband GPIO for wakeup instead
>>>>>      *		   of inband wake support (such as sdx24)
>>>>> + * @pm_disable: disables runtime PM (optional)
>>>>>      */
>>>>>     struct mhi_pci_dev_info {
>>>>>     	const struct mhi_controller_config *config;
>>>>> @@ -54,6 +55,7 @@ struct mhi_pci_dev_info {
>>>>>     	unsigned int dma_data_width;
>>>>>     	unsigned int mru_default;
>>>>>     	bool sideband_wake;
>>>>> +	bool pm_disable;
>>>>>     };
>>>>>     #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \
>>>>> @@ -295,6 +297,7 @@ static const struct mhi_pci_dev_info mhi_qcom_qdu100_info = {
>>>>>     	.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
>>>>>     	.dma_data_width = 32,
>>>>>     	.sideband_wake = false,
>>>>> +	.pm_disable = true,
>>>>
>>>> |no_m3|orno_|m3_support|would be more suitable than|pm_disable|
>>>
>>> Yes!
>>>
>>> But does the device not supporting M3 only or D3Hot also? If the former, then we
>>> should prevent MHI host to enter/exit M3 state in mhi_pm_suspend/ mhi_pm_resume.
>>> There is an incentive in allowing D3Hot if the device supports it.
>>>
>>> Also, is there a way we could probe M3 support in the device so that we can
>>> check it during runtime?
>>>
>>> - Mani
>>>
>> In QDU100 device does not support M3 state, D3_hot will be supported. As
>> QDU100 is an accelerator card which needs high-throughout, LPM needs to
>> disabled here. So we are trying to disable runtime PM here, without M3 ,
>> D3_hot will not have effect in QDU100.
>> Also in MHI we don't have provision to check device M3 capability, so we are
>> trying to disable runtime PM here.
>>
> 
> Even though PCI core is enabling runtime PM for devices, it also prevents the
> devices from getting suspended unless the drivers explicitly asks for it. So
> this driver is using autosuspend feature to allow the devices to be suspended
> if PME is supported from D3Hot.
> 
> If you want to prevent the device from getting runtime suspended, you should
> just skip autosuspend.
> 
> - Mani
> 
Thank you, I will upload new patchset accordingly.