On Thu, Nov 02, 2017 at 09:55:40AM +0100, Maarten Lankhorst wrote: > Op 01-11-17 om 18:00 schreef Ville Syrjälä: > > On Wed, Nov 01, 2017 at 04:55:06PM +0100, Maarten Lankhorst wrote: > >> Op 01-11-17 om 16:29 schreef Ville Syrjälä: > >>> On Wed, Nov 01, 2017 at 04:04:33PM +0100, Maarten Lankhorst wrote: > >>>> This introduces a slight behavioral change to rmfb. Instead of > >>>> disabling a crtc when the primary plane is disabled, we try to > >>>> preserve it. > >>>> > >>>> Apart from old versions of the vmwgfx xorg driver, there is > >>>> nothing depending on rmfb disabling a crtc. > >>>> > >>>> Vmwgfx' and simple kms helper atomic implementation rejects CRTC > >>>> enabled without plane, so we can do this safely. > > The code for those seems a bit inconsistent. The crtc check requires > > that the crtc state and plane state match. But the plane check allows > > the plane to be enabled w/o the crtc being enabled. I guess it doesn't > > matter really since you can't enable the plane without a crtc, and the > > crtc check would then catch the case where the crtc would be disabled. > > Exactly. :-) Only when a plane is unbound and stays unbound, the crtc check won't > be invoked. Hence it's the most accurate way of making sure that > crtc enabled <=> primary plane bound. > > If the check was done in the primary plane, an atomic modeset could enable > the crtc without enabling the primary plane, which shouldn't be allowed but > a plane check won't catch it. > > This has been a bug in simple-kms-helper, fixed in the below commit: > > commit 765831dc27ab141b3a0be1ab55b922b012427902 > Author: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > Date: Wed Jul 12 10:13:29 2017 +0200 > > drm/simple-kms-helper: Fix the check for the mismatch between plane and CRTC enabled. Hmm. OK that part looks OK. What does seem a bit inconsistent is the fact that we pass can_update_disabled=true, but later on we reject the update when visible==false. The old code would have accepted that because it didn't even call drm_plane_helper_check_state() when the crtc (and thus also the plane) was disabled. -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
