On Thu, Jul 11, 2013 at 6:53 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> On Thu, 11 Jul 2013 16:02:27 +0800
> Xiong Zhang <xiong.y.zhang at intel.com> wrote:
>
>> display.crtc_mode_set will increase pll->refcount, but no one will
>> decrease pll->refcount when freeze gpu. So when gpu resume from freeze,
>> pll->refcount is still larger than zero. This is abnormal
>>
>> Without this patch, connecting vga screen on Haswell platform, there
>> are following results:
>> 1. when resume S3, call trace exist in intel_ddi_put_crtc_pll()
>> 2. when resume s4, vga monitor is black. because intel_ddi_pll_mode_set()
>> return false and haswell_crtc_mode_set() exit without setting mode
>>
>> With this patch, I don't find S3 and S4 regression on SandyBridge
>> and IvyBridge platform connecting VGA, HDMI and DP screen.
>>
>> Signed-off-by: Xiong Zhang <xiong.y.zhang at intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 0485f43..0065735 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -575,8 +575,10 @@ static int i915_drm_freeze(struct drm_device *dev)
>> * Disable CRTCs directly since we want to preserve sw state
>> * for _thaw.
>> */
>> - list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
>> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
>> dev_priv->display.crtc_disable(crtc);
>> + dev_priv->display.off(crtc);
>> + }
>>
>> intel_modeset_suspend_hw(dev);
>> }
>
> The comment above this call indicates we'll trash the sw state if we
> call ->off directly. Does suspend/resume still work both with and
> without X with this patch applied? If we trash the sw state, the VT
> switchless resume shouldn't work...
Even without that little issue: ddi refcounting issue need to be fixed
in the haswell platform code, not by papering over in the core modeset
infrastructure. We have refcounted pch plls on snb/ivb, and it works.
So imo there's no issue with the core code in the driver.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
