On Wed, Mar 04, 2015 at 10:49:04AM -0800, Matt Roper wrote:
> We need to disable all sprite planes when disabling the CRTC. We had
> been using the top-level atomic 'disable' entrypoint to accomplish this,
> which was wrong. Not only can this lead to various locking issues, it
> also modifies the actual plane state, making it impossible to restore
> the plane properly later. For example, a DPMS off followed by a DPMS on
> will result in any sprite planes in use not being restored properly.
>
> The proper solution here is to call directly into our 'commit plane'
> hook with a copy of the plane's current state that has 'visible' set to
> false. Committing this dummy state will turn off the plane, but will
> not touch the actual plane->state pointer, allowing us to properly
> restore the plane state later.
>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
> This was a pretty big problem so I suspect this is probably tied to some of our
> existing bugzilla's but I'm not sure exactly which ones. It seems that we lack
> an i-g-t test for plane status across DPMS, so I'll send along a new i-g-t test
> for that shortly.
>
> drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 92cb2ff..6277701 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4201,6 +4201,24 @@ static void intel_enable_sprite_planes(struct drm_crtc *crtc)
> }
> }
>
> +/*
> + * Disable a plane internally without actually modifying the plane's state.
> + * This will allow us to easily restore the plane later by just reprogramming
> + * its state.
> + */
> +static void disable_plane_internal(struct drm_plane *plane)
> +{
> + struct intel_plane *intel_plane = to_intel_plane(plane);
> + struct drm_plane_state *state =
> + plane->funcs->atomic_duplicate_state(plane);
> + struct intel_plane_state *intel_state = to_intel_plane_state(state);
> +
> + intel_state->visible = false;
> + intel_plane->commit_plane(plane, intel_state);
> +
> + intel_plane_destroy_state(plane, state);
I wonder whether long-term we should do a full atomic commit including the
begin/end dance here. But this is more than good enough for now, so merged
to dinq.
Thanks, Daniel
> +}
> +
> static void intel_disable_sprite_planes(struct drm_crtc *crtc)
> {
> struct drm_device *dev = crtc->dev;
> @@ -4210,8 +4228,8 @@ static void intel_disable_sprite_planes(struct drm_crtc *crtc)
>
> drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
> intel_plane = to_intel_plane(plane);
> - if (intel_plane->pipe == pipe)
> - plane->funcs->disable_plane(plane);
> + if (plane->fb && intel_plane->pipe == pipe)
> + disable_plane_internal(plane);
> }
> }
>
> --
> 1.8.5.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
