Re: [PATCH 03/12] PCI/ACPI: Add aux power grant notifier

["Nilawar, Badal" <badal.nilawar@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2736 bytes --]


On 08-04-2025 18:31, Rafael J. Wysocki wrote:
> On Fri, Apr 4, 2025 at 2:53 PM Gupta, Anshuman<anshuman.gupta@intel.com> wrote:
>> /snip
>>>>> Exactly like I said: If you only allow one driver to use the _DSM to
>>>>> request the Aux power from a given Root Port, it will have all of
>>>>> the information and does not need to be notified about any changes.
>>>>> Since no one else is allowed to use this interface for that Root
>>>>> Port, no one else will need a notifier either.  For this to work,
>>>>> you need some mechanism allowing drivers to claim the interface so
>>>>> no one else can use it (on a per Root Port basis) which is currently missing
>>> AFAICS.
>>>> IMHO such kind of mechanism will require to add Root Port specific
>>>> data structure to claim the interface. But real problem is the criteria  to claim
>>> the interface.
>>>> Is first PCIe Non-Bridge Endpoint Function 0 driver can be criteria
>>>> to claim the Interface. Or first come and first serve approach ?
>>> IMV, the first PCIe Non-Bridge Endpoint Function 0 driver approach would be
>>> sort of fragile and cumbersome to enforce.
>>>
>>> First come, first serve is much simpler and should be sufficient for now AFAICS.
>> We are enabling VRSR only for default vga boot device.
>> As it needed only GPU driving the display for better user experience.
>> Can we use same logic vga_default_device() to claim the interface under root port.
>> That will simplify the criteria to claim the interface.
> Basically, you need to prevent somebody else from running
> DSM_PCI_D3COLD_AUX_POWER_LIMIT concurrently for the given Root Port
> and store the information that it has been run already.
>
> Personally, I'd add aux_power_limit to struct acpi_device_power and
> I'd use a static mutex in pci_acpi_request_d3cold_aux_power() along
> the lines of:
>
> 1. Acquire the mutex.
> 2. If power.aux_power_limit is set for the ACPI companion of pci_dev,
> release the mutex and bail out.
> 3. Evaluate DSM_PCI_D3COLD_AUX_POWER_LIMIT and if it fails, release
> the mutex and bail out.
> 4. Set power.aux_power_limit for the ACPI companion of pci_dev to the
> requested value.
> 5. Release the mutex.
>
> Of course, this would only allow it to be set once per kernel boot, so
> in order to handle hibernation properly, the same Aux power limit
> would need to be requested again automatically on Root Port restore.

  I've incorporated the above suggestion in v4 patch [v4,02/11] 
PCI/ACPI: Per root port allow one Aux power limit request - Patchwork 
<https://patchwork.freedesktop.org/patch/656187/?series=145342&rev=4> , 
except for the Aux power re-request after hibernation exit, as we didn't 
observe any issues with VRSR post hibernation.

Regards,
Badal

[-- Attachment #2: Type: text/html, Size: 4131 bytes --]