Re: [PATCH v6 06/12] drm/xe/vrsr: Enable VRSR on default VGA boot device

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

On 20-01-2026 20:12, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: Jani Nikula <jani.nikula@linux.intel.com>
>> Sent: Tuesday, January 20, 2026 7:14 PM
>> To: Nilawar, Badal <badal.nilawar@intel.com>; Vivi, Rodrigo
>> <rodrigo.vivi@intel.com>
>> Cc: intel-xe@lists.freedesktop.org; linux-acpi@vger.kernel.org; linux-
>> pci@vger.kernel.org; Gupta, Anshuman <anshuman.gupta@intel.com>;
>> rafael@kernel.org; lenb@kernel.org; bhelgaas@google.com;
>> ilpo.jarvinen@linux.intel.com; Gupta, Varun <varun.gupta@intel.com>;
>> ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Poosa,
>> Karthik <karthik.poosa@intel.com>; Auld, Matthew <matthew.auld@intel.com>;
>> Anirban, Sk <sk.anirban@intel.com>; Jadav, Raag <raag.jadav@intel.com>
>> Subject: Re: [PATCH v6 06/12] drm/xe/vrsr: Enable VRSR on default VGA boot
>> device
>>
>> On Tue, 20 Jan 2026, "Nilawar, Badal" <badal.nilawar@intel.com> wrote:
>>> On 15-01-2026 20:55, Rodrigo Vivi wrote:
>>>> On Thu, Jan 15, 2026 at 04:25:06PM +0200, Jani Nikula wrote:
>>>>> On Tue, 13 Jan 2026, Badal Nilawar <badal.nilawar@intel.com> wrote:
>>>>>> The VRSR feature is to enhance the display screen refresh
>>>>>> experience when the device exits from the D3cold state. Therefore,
>>>>>> apply the VRSR feature to the default VGA boot device and when a display
>> is connected.
>>>>> I don't understand how you get from the 1st sentence "therefore" the
>>>>> 2nd sentence. Please elaborate what you're trying to do here, and why.
>>>> On a scenario with multiple GPU, only one can use the aux power and
>>>> the feature itself was mainly designed for the display case - to
>>>> bring up display faster after the d3cold.
>>> This is to enhance screen refresh experience of primary display.
>> The way it's being added, it's just really oddly specific.
>>
>>>> But yes, the right explanation for the why needs to be here.
>>> I will rephrase the explanation.
>>>> Also, although unlikely, we never know what users can do out there,
>>>> and perhaps we will have someone with multiple cards and display
>>>> plugged in more than one?! We probably also need a global
>>>> counter/flag to avoid a second one to quick in.
>>>>
>>>> But we definitely need to prioritize the first one with display connected.
>>> At present there is no way to know which one is primary display that's
>>> why check is against default_vga_device.
>>>>> So we have xe_pci_probe() -> xe_pm_init() -> xe_pm_vrsr_init() ->
>>>>> xe_display_connected() -> intel_display_connected(), and that's the
>>>>> only path and point in time to check whether displays are connected.
>>>>> If not, the decision is "not VRSR capable", which is just a weird
>>>>> concusion to make. The *capability* does not depend on displays, does it?
>>>>>
>>>>> If you boot a device without a display, and then plug in a display,
>>>>> no VRSR for you?
>>>> yeap, it looks like the check is in the wrong place. It needs to be
>>>> checked when going to d3cold...
>>> Yes, VRSR will not be enabled if display is not connected at boot.
>> Why? And this needs to be properly explained in the commit message. The
>> current one isn't enough.
>>
>>> *capability* does not depend on display but VRSR use case is.
>> Please at least don't conflate the two. Don't determine capability based on whether
>> the conditions on the use case exist.
>>
>> Contrast with, I don't know, FBC. The platform will still have FBC capability even if
>> the conditions for enabling it aren't met.
> Yes right, I think display can be plugged later after boot as well. In this case also VRSR should be
> enabled.  This can be handled through the display hotplug path and VRSR can be enabled accordingly.
Sure will add handling in display hotplug path.
>
> Also elaborate the reasoning so the assumptions, limitations and design choices are clear and
> why certain trade offs are made are clarified.

Sure.

Thanks,
Badal

>
> Regards,
> Uma Shankar
>
>> BR,
>> Jani.
>>
>>
>>>>> More comments inline.
>>>>>
>>>>>> v2: Move generic display logic to i915/display (Jani)
>>>>>>
>>>>>> Signed-off-by: Badal Nilawar <badal.nilawar@intel.com>
>>>>>> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
>>>>>> ---
>>>>>>    drivers/gpu/drm/i915/display/intel_display.c | 22 ++++++++++++++++++++
>>>>>>    drivers/gpu/drm/i915/display/intel_display.h |  1 +
>>>>>>    drivers/gpu/drm/xe/display/xe_display.c      |  5 +++++
>>>>>>    drivers/gpu/drm/xe/display/xe_display.h      |  2 ++
>>>>>>    drivers/gpu/drm/xe/xe_pm.c                   |  5 +++++
>>>>>>    5 files changed, 35 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>>>>>> b/drivers/gpu/drm/i915/display/intel_display.c
>>>>>> index 81b3a6692ca2..97c74272fb19 100644
>>>>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>>>>> @@ -8426,3 +8426,25 @@ bool intel_scanout_needs_vtd_wa(struct
>> intel_display *display)
>>>>>>    {
>>>>>>    	return IS_DISPLAY_VER(display, 6, 11) &&
>> intel_display_vtd_active(display);
>>>>>>    }
>>>>>> +
>>>>>> +bool intel_display_connected(struct intel_display *display) {
>>>>>> +	struct drm_connector *list_connector;
>>>>>> +	struct drm_connector_list_iter iter;
>>>>>> +	bool ret = false;
>>>>>> +
>>>>>> +	mutex_lock(&display->drm->mode_config.mutex);
>>>>>> +	drm_connector_list_iter_begin(display->drm, &iter);
>>>>>> +
>>>>>> +	drm_for_each_connector_iter(list_connector, &iter) {
>>>>>> +		if (list_connector->status == connector_status_connected) {
>>>>>> +			ret = true;
>>>>>> +			break;
>>>>>> +		}
>>>>>> +	}
>>>>>> +
>>>>>> +	drm_connector_list_iter_end(&iter);
>>>>>> +	mutex_unlock(&display->drm->mode_config.mutex);
>>>>>> +
>>>>>> +	return ret;
>>>>>> +}
>>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.h
>>>>>> b/drivers/gpu/drm/i915/display/intel_display.h
>>>>>> index f8e6e4e82722..20690aa59324 100644
>>>>>> --- a/drivers/gpu/drm/i915/display/intel_display.h
>>>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.h
>>>>>> @@ -560,5 +560,6 @@ bool assert_port_valid(struct intel_display
>>>>>> *display, enum port port);
>>>>>>
>>>>>>    bool intel_scanout_needs_vtd_wa(struct intel_display *display);
>>>>>>    int intel_crtc_num_joined_pipes(const struct intel_crtc_state
>>>>>> *crtc_state);
>>>>>> +bool intel_display_connected(struct intel_display *display);
>>>>>>
>>>>>>    #endif
>>>>>> diff --git a/drivers/gpu/drm/xe/display/xe_display.c
>>>>>> b/drivers/gpu/drm/xe/display/xe_display.c
>>>>>> index f8a831b5dc7d..54ed39b257ad 100644
>>>>>> --- a/drivers/gpu/drm/xe/display/xe_display.c
>>>>>> +++ b/drivers/gpu/drm/xe/display/xe_display.c
>>>>>> @@ -64,6 +64,11 @@ bool xe_display_driver_probe_defer(struct pci_dev
>> *pdev)
>>>>>>    	return intel_display_driver_probe_defer(pdev);
>>>>>>    }
>>>>>>
>>>>>> +bool xe_display_connected(struct xe_device *xe) {
>>>>>> +	return intel_display_connected(xe->display);
>>>>>> +}
>>>>>> +
>>>>>>    /**
>>>>>>     * xe_display_driver_set_hooks - Add driver flags and hooks for display
>>>>>>     * @driver: DRM device driver
>>>>>> diff --git a/drivers/gpu/drm/xe/display/xe_display.h
>>>>>> b/drivers/gpu/drm/xe/display/xe_display.h
>>>>>> index 76db95c25f7e..11d4b09808e5 100644
>>>>>> --- a/drivers/gpu/drm/xe/display/xe_display.h
>>>>>> +++ b/drivers/gpu/drm/xe/display/xe_display.h
>>>>>> @@ -37,6 +37,7 @@ void xe_display_pm_resume(struct xe_device *xe);
>>>>>>    void xe_display_pm_runtime_suspend(struct xe_device *xe);
>>>>>>    void xe_display_pm_runtime_suspend_late(struct xe_device *xe);
>>>>>>    void xe_display_pm_runtime_resume(struct xe_device *xe);
>>>>>> +bool xe_display_connected(struct xe_device *xe);
>>>>>>
>>>>>>    #else
>>>>>>
>>>>>> @@ -67,5 +68,6 @@ static inline void
>> xe_display_pm_runtime_suspend(struct xe_device *xe) {}
>>>>>>    static inline void xe_display_pm_runtime_suspend_late(struct xe_device
>> *xe) {}
>>>>>>    static inline void xe_display_pm_runtime_resume(struct xe_device
>>>>>> *xe) {}
>>>>>>
>>>>>> +static inline bool xe_display_connected(struct xe_device *xe) {
>>>>>> +return false; }
>>>>> There was a blank line before #endif. Please keep it. Ditto
>>>>> throughout the series.
>>>>>
>>>>>>    #endif /* CONFIG_DRM_XE_DISPLAY */
>>>>>>    #endif /* _XE_DISPLAY_H_ */
>>>>>> diff --git a/drivers/gpu/drm/xe/xe_pm.c
>>>>>> b/drivers/gpu/drm/xe/xe_pm.c index 3fe673f0f87d..e7aa876ce9e0
>>>>>> 100644
>>>>>> --- a/drivers/gpu/drm/xe/xe_pm.c
>>>>>> +++ b/drivers/gpu/drm/xe/xe_pm.c
>>>>>> @@ -9,6 +9,7 @@
>>>>>>    #include <linux/fault-inject.h>
>>>>>>    #include <linux/pm_runtime.h>
>>>>>>    #include <linux/suspend.h>
>>>>>> +#include <linux/vgaarb.h>
>>>>>>
>>>>>>    #include <drm/drm_managed.h>
>>>>>>    #include <drm/ttm/ttm_placement.h> @@ -387,6 +388,7 @@ static int
>>>>>> pci_acpi_aux_power_setup(struct xe_device *xe)
>>>>>>
>>>>>>    static void xe_pm_vrsr_init(struct xe_device *xe)
>>>>>>    {
>>>>>> +	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>>>>>>    	int ret;
>>>>>>
>>>>>>    	if (!xe->info.has_vrsr)
>>>>>> @@ -395,6 +397,9 @@ static void xe_pm_vrsr_init(struct xe_device *xe)
>>>>>>    	if (!xe_pm_vrsr_capable(xe))
>>>>>>    		return;
>>>>>>
>>>>>> +	if (pdev != vga_default_device() || !xe_display_connected(xe))
>>>>> Simply considering the places in the kernel that call
>>>>> vga_default_device(), this just doesn't feel right.
>>>> I also don't understand why to check this vga default device...
>>> As previously mentioned, a check for the default VGA device was added
>>> to determine if this is the primary display.
>>>
>>> Thanks,
>>> Badal
>>>
>>>>> BR,
>>>>> Jani.
>>>>>
>>>>>
>>>>>> +		return;
>>>>>> +
>>>>>>    	/*
>>>>>>    	 * If the VRSR initialization fails, the device will proceed with the regular
>>>>>>    	 * D3cold flow
>>>>> --
>>>>> Jani Nikula, Intel
>> --
>> Jani Nikula, Intel