On Tue, Jul 14, 2015 at 12:30:38PM +0200, Maarten Lankhorst wrote: > Op 14-07-15 om 11:53 schreef Daniel Vetter: > > On Mon, Jul 13, 2015 at 04:30:20PM +0200, Maarten Lankhorst wrote: > >> All non-primary planes get disabled during hw readout, > >> this reduces complexity and means not having to do some plane > >> visibility checks during the first commit. > >> > >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > > I still think calling readout_plane_state from the hw state readout code > > is the wrong approach. Instead we should consolidate all the plane > > readout code exclusively into a new intel_plane_readout_hw_state helper > > which is called only from driver load paths. That should also contain the > > fb/gem_bo reconstruction loop. > Is that a nack? Nope, just a "I think we want to clarify this more". I'd welcome a patch on top, but can be done as part of converting dpms (since that will result in lots of cleanups too). > > For the other 2 users of this code (lid notifiery and resume) we should > > just force an unconditional plane restore by setting > > crtc_state->planes_chagned. But I thinkt hat's ok as some follow-up work, > > once atomic has settled a bit more. > > > If you want to force a plane restore, planes_changed won't do what you think it does. > planes_changed is a flag that says one or more planes were added to the crtc_state. > > You want to do drm_atomic_add_affected_planes for that, and atomic resume would do that > since it duplicates all plane state. Ah right, so should be possible to just consolidate the plane readout code without requiring any changes in the force_restore case. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
