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@xxxxxxxxxxx> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > Cc: Inki Dae <inki.dae@xxxxxxxxxxx> > Cc: Matt Roper <matthew.d.roper@xxxxxxxxx> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> Tested-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> (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); > } -- Regards, Laurent Pinchart _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
