[Intel-gfx] [PATCH 04/11] drm/i915: get runtime PM at intel_set_mode

[Paulo Zanoni]

2014-02-24 8:23 GMT-03:00 Imre Deak <imre.deak at intel.com>:
> On Fri, 2014-02-21 at 13:52 -0300, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>>
>> Otherwise, when we run intel_modeset_check_state we may already be
>> runtime suspended, and our state checking code will read registers
>> while the device is suspended. This can only happen if your
>> autosuspend_delay_ms is low (not the default 10s).
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 10ec401..c64fb7f 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -9746,13 +9746,18 @@ static int intel_set_mode(struct drm_crtc *crtc,
>>                         struct drm_display_mode *mode,
>>                         int x, int y, struct drm_framebuffer *fb)
>>  {
>> +     struct drm_device *dev = crtc->dev;
>> +     struct drm_i915_private *dev_priv = dev->dev_private;
>>       int ret;
>>
>> +     intel_runtime_pm_get(dev_priv);
>> +
>>       ret = __intel_set_mode(crtc, mode, x, y, fb);
>>
>>       if (ret == 0)
>>               intel_modeset_check_state(crtc->dev);
>>
>> +     intel_runtime_pm_put(dev_priv);
>>       return ret;
>>  }
>
> Ideally these should be done as part of a power domain get/put as some
> platforms will need to turn on some power wells too and on that path we
> do anyway a runtime PM get/put.
>
> In the latest VLV power domain support patchset [1] I added the power
> domain get/put and state check to places I thought necessary. I haven't
> tested it on HSW but afaics the ones added for the HW state readout code
> would solve the issue you describe here.

Yes. I just quickly read the patches, and they seem to try to solve
this problem. Due to the reasons you wrote on the first paragraph, I
think in the long term we want the power domains solution. But as I
mentioned in the cover letter, this series contains bug fixes and
maybe we want them on -fixes and even stable Kernels, so maybe we want
to merge this patch, then later merge the code that uses power
domains, then remove the runitme_pm_get calls and leave just the power
domain calls? I'm not sure.

>
> --Imre
>
> [1] http://www.spinics.net/lists/intel-gfx/msg40344.html



-- 
Paulo Zanoni