[Intel-gfx] [PATCH] drm/i915: Hold runtime PM during plane commit

[Paulo Zanoni]

2014-12-15 16:11 GMT-02:00 Matt Roper <matthew.d.roper at intel.com>:
> 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 at intel.com>
>         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 at intel.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87180
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>

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 at intel.com>

> ---
>  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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni