2014-12-15 16:11 GMT-02:00 Matt Roper <matthew.d.roper@xxxxxxxxx>: > During plane operations, we read/write some registers that only operate > properly if we're not runtime suspended. At the moment we're not > holding the runtime PM reference across the whole plane operation, so > there's a potential for problems. > > This issue was already partially addressed by commit > > commit d6dd6843ff4a57c662dbc378b9f99a9c034b0956 > Author: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > Date: Fri Aug 15 15:59:32 2014 -0300 > > drm/i915: fix plane/cursor handling when runtime suspended > > which took care of holding the runtime PM reference during the pin and > fence operations for plane updates. However there are still a few > actual plane registers that we also need to hold the runtime PM > reference for. Recent refactoring patches in preparation for atomic > have rearranged the code and made it increasingly likely that the > hardware will have time to suspend between the pin/fence operation and > the actual register writes. > > The solution here grabs the runtime PM reference around the 'commit' > operation for planes, which should cover all the relevant register > reads/writes. > > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87180 > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> I see we're in the middle of a very big rework on how all these prepare/commit functions are called, so I don't think it makes sense to spend too much time trying to find the very-best-perfect spot for the get/put calls, since they're likely to be changed later. So I guess that for now it's important to fix the current "regression" reported by QA: Testcase: igt/pm-rpm/legacy-planes Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_display.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 3044af5..a0ddce5 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11863,6 +11863,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > uint32_t src_w, uint32_t src_h) > { > struct drm_device *dev = plane->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_framebuffer *old_fb = plane->fb; > struct intel_plane_state state; > struct intel_plane *intel_plane = to_intel_plane(plane); > @@ -11902,7 +11903,9 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, > return ret; > } > > + intel_runtime_pm_get(dev_priv); > intel_plane->commit_plane(plane, &state); > + intel_runtime_pm_put(dev_priv); > > if (fb != old_fb && old_fb) { > if (intel_crtc->active) > -- > 1.8.5.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
