Hi Daniel,
Thank you for the patch.
On Friday 10 April 2015 16:22:39 Daniel Vetter wrote:
It's a silly thing to do and surprises driver writers. Most likely this did already blow up for exynos.
It's also a silly thing to change plane state when it's off, but fbdev is silly (it does an unconditional modeset over all planes). And userspace can be evil. So I think we need this.
With this check in the helpers we can remove the one in i915 code for the same conditions (becuase ->crtc iff ->fb).
Cc: Gustavo Padovan gustavo@padovan.org Cc: dri-devel@lists.freedesktop.org Cc: Inki Dae inki.dae@samsung.com Cc: Matt Roper matthew.d.roper@intel.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com Tested-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
(with the ongoing omapdrm atomic update conversion work)
drivers/gpu/drm/drm_atomic_helper.c | 3 ++- drivers/gpu/drm/i915/intel_atomic_plane.c | 4 ---- 2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 41c38edade74..e1556143d811 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -1216,7 +1216,8 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev, if (drm_atomic_plane_disabling(plane, old_plane_state) && funcs->atomic_disable) funcs->atomic_disable(plane, old_plane_state);
else
else if (plane->state->crtc ||drm_atomic_plane_disabling(plane, old_plane_state)) funcs->atomic_update(plane, old_plane_state);
The test is so trivial that I wonder whether it makes sense to make atomic_disable() optional. Wouldn't it be easier to either make atomic_disable() mandatory, or to remove it completely ?
}
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c index 976b89156570..cb383a0fc392 100644 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -172,10 +172,6 @@ static void intel_plane_atomic_update(struct drm_plane *plane, struct intel_plane_state *intel_state = to_intel_plane_state(plane->state);
- /* Don't disable an already disabled plane */
- if (!plane->state->fb && !old_state->fb)
return;- intel_plane->commit_plane(plane, intel_state);
}