Re: [PATCH 04/11] drm/i915: get runtime PM at intel_set_mode

[Imre Deak <imre.deak@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2768 bytes --]

On Mon, 2014-02-24 at 11:34 -0300, Paulo Zanoni wrote:
> 2014-02-24 8:23 GMT-03:00 Imre Deak <imre.deak@intel.com>:
> > On Fri, 2014-02-21 at 13:52 -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@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@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.

Ok, I can rebase my patchset based on the above.

Note that runtime PM was enabled post 3.13, so this fix is needed for
-fixes, but not for stable kernels.

The same goes for patch 5/11.

--Imre

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx