drm: add set_connectors to Surface

src/backend/drm/atomic/session.rs

@@ -49,7 +49,9 @@ impl<A: AsRawFd + 'static> SessionObserver for AtomicDrmDeviceObserver<A> {
                 return;
             }
         }
+
         // TODO: Clear overlay planes (if we ever use them)
+
         if let Some(backends) = self.backends.upgrade() {
             for surface in backends.borrow().values().filter_map(Weak::upgrade) {
                 // other ttys that use no cursor, might not clear it themselves.

src/backend/drm/atomic/surface.rs

@@ -181,11 +181,15 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurfaceInternal<A> {
 
         // check if the connector can handle the current mode
         if info.modes().contains(pending.mode.as_ref().unwrap()) {
-            let mut conns = pending.connectors.clone();
-            conns.insert(conn);
-
             // check if config is supported
-            let req = self.build_request(&conns, &self.planes, None, pending.mode, pending.blob)?;
+            let req = self.build_request(
+                &mut [conn].iter(),
+                &mut [].iter(),
+                &self.planes,
+                None,
+                pending.mode,
+                pending.blob,
+            )?;
             self.atomic_commit(
                 &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
                 req,
@@ -194,7 +198,7 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurfaceInternal<A> {
             .map_err(|_| Error::TestFailed(self.crtc))?;
 
             // seems to be, lets add the connector
-            pending.connectors = conns;
+            pending.connectors.insert(conn);
 
             Ok(())
         } else {
@@ -202,15 +206,18 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurfaceInternal<A> {
         }
     }
 
-    fn remove_connector(&self, connector: connector::Handle) -> Result<(), Error> {
+    fn remove_connector(&self, conn: connector::Handle) -> Result<(), Error> {
         let mut pending = self.pending.write().unwrap();
 
-        // remove it temporary
-        let mut conns = pending.connectors.clone();
-        conns.remove(&connector);
-
         // check if new config is supported (should be)
-        let req = self.build_request(&conns, &self.planes, None, pending.mode, pending.blob)?;
+        let req = self.build_request(
+            &mut [].iter(),
+            &mut [conn].iter(),
+            &self.planes,
+            None,
+            pending.mode,
+            pending.blob,
+        )?;
         self.atomic_commit(
             &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
             req,
@@ -219,37 +226,37 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurfaceInternal<A> {
         .map_err(|_| Error::TestFailed(self.crtc))?;
 
         // seems to be, lets remove the connector
+        pending.connectors.remove(&conn);
+
+        Ok(())
+    }
+
+    fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Error> {
+        let current = self.state.write().unwrap();
+        let mut pending = self.pending.write().unwrap();
+
+        let conns = connectors.iter().cloned().collect::<HashSet<_>>();
+        let mut added = conns.difference(&current.connectors);
+        let mut removed = current.connectors.difference(&conns);
+
+        let req = self.build_request(
+            &mut added,
+            &mut removed,
+            &self.planes,
+            None,
+            pending.mode,
+            pending.blob,
+        )?;
+
+        self.atomic_commit(
+            &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
+            req,
+        )
+        .map_err(|_| Error::TestFailed(self.crtc))?;
+
         pending.connectors = conns;
 
-        // try to disable it
-        let mut req = AtomicModeReq::new();
-
-        req.add_property(
-            connector,
-            self.conn_prop_handle(connector, "CRTC_ID")?,
-            property::Value::CRTC(None),
-        );
-
-        if let Err(err) = self
-            .atomic_commit(&[AtomicCommitFlags::TestOnly], req.clone())
-            .compat()
-            .map_err(|_| Error::TestFailed(self.crtc))
-        {
-            warn!(
-                self.logger,
-                "Could not disable connector ({:?}) (but rendering will be stopped): {}", connector, err
-            );
         Ok(())
-        } else {
-            // should succeed, any error is serious
-            self.atomic_commit(&[AtomicCommitFlags::Nonblock], req.clone())
-                .compat()
-                .map_err(|source| Error::Access {
-                    errmsg: "Failed to commit disable connector",
-                    dev: self.dev_path(),
-                    source,
-                })
-        }
     }
 
     fn use_mode(&self, mode: Option<Mode>) -> Result<(), Error> {
@@ -269,7 +276,14 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurfaceInternal<A> {
             None => property::Value::Unknown(0),
         });
 
-        let req = self.build_request(&pending.connectors, &self.planes, None, mode, new_blob)?;
+        let req = self.build_request(
+            &mut pending.connectors.iter(),
+            &mut [].iter(),
+            &self.planes,
+            None,
+            mode,
+            new_blob,
+        )?;
         if let Err(err) = self
             .atomic_commit(
                 &[AtomicCommitFlags::AllowModeset, AtomicCommitFlags::TestOnly],
@@ -304,13 +318,12 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
             "Preparing Commit.\n\tCurrent: {:?}\n\tPending: {:?}\n", *current, *pending
         );
 
-        {
         let current_conns = current.connectors.clone();
         let pending_conns = pending.connectors.clone();
-            let removed = current_conns.difference(&pending_conns);
+        let mut removed = current_conns.difference(&pending_conns);
-            let added = pending_conns.difference(&current_conns);
+        let mut added = pending_conns.difference(&current_conns);
 
-            for conn in removed {
+        for conn in removed.clone() {
             if let Ok(info) = self.get_connector(*conn) {
                 info!(self.logger, "Removing connector: {:?}", info.interface());
             } else {
@@ -318,7 +331,7 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
             }
         }
 
-            for conn in added {
+        for conn in added.clone() {
             if let Ok(info) = self.get_connector(*conn) {
                 info!(self.logger, "Adding connector: {:?}", info.interface());
             } else {
@@ -333,12 +346,13 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
                 pending.mode.as_ref().unwrap().name()
             );
         }
-        }
 
         trace!(self.logger, "Testing screen config");
-        {
+
+        let req = {
             let req = self.build_request(
-                &pending.connectors,
+                &mut added,
+                &mut removed,
                 &self.planes,
                 Some(framebuffer),
                 pending.mode,
@@ -360,6 +374,15 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
                 info!(self.logger, "Reverting back to last know good state");
 
                 *pending = current.clone();
+
+                self.build_request(
+                    &mut [].iter(),
+                    &mut [].iter(),
+                    &self.planes,
+                    Some(framebuffer),
+                    current.mode,
+                    current.blob,
+                )?
             } else {
                 if current.mode != pending.mode {
                     if let Some(blob) = current.blob {
@@ -369,16 +392,12 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
                     }
                 }
                 *current = pending.clone();
-            }
-        }
 
-        let req = self.build_request(
+                // new config
-            &current.connectors,
+                req
-            &self.planes,
+            }
-            Some(framebuffer),
+        };
-            current.mode,
+
-            current.blob,
-        )?;
         debug!(self.logger, "Setting screen: {:#?}", req);
         self.atomic_commit(
             &[
@@ -399,9 +418,15 @@ impl<A: AsRawFd + 'static> RawSurface for AtomicDrmSurfaceInternal<A> {
     }
 
     fn page_flip(&self, framebuffer: framebuffer::Handle) -> Result<(), SwapBuffersError> {
-        let current = self.state.read().unwrap();
         let req = self
-            .build_request(&current.connectors, &self.planes, Some(framebuffer), None, None) //current.mode)
+            .build_request(
+                &mut [].iter(),
+                &mut [].iter(),
+                &self.planes,
+                Some(framebuffer),
+                None,
+                None,
+            ) //current.mode)
             .map_err(|_| SwapBuffersError::ContextLost)?;
         trace!(self.logger, "Queueing page flip: {:#?}", req);
         self.atomic_commit(
@@ -530,7 +555,8 @@ impl<A: AsRawFd + 'static> AtomicDrmSurfaceInternal<A> {
     // If a mode is set a matching blob needs to be set (the inverse is not true)
     fn build_request(
         &self,
-        connectors: &HashSet<connector::Handle>,
+        new_connectors: &mut dyn Iterator<Item = &connector::Handle>,
+        removed_connectors: &mut dyn Iterator<Item = &connector::Handle>,
         planes: &Planes,
         framebuffer: Option<framebuffer::Handle>,
         mode: Option<Mode>,
@@ -538,7 +564,7 @@ impl<A: AsRawFd + 'static> AtomicDrmSurfaceInternal<A> {
     ) -> Result<AtomicModeReq, Error> {
         let mut req = AtomicModeReq::new();
 
-        for conn in connectors.iter() {
+        for conn in new_connectors {
             req.add_property(
                 *conn,
                 self.conn_prop_handle(*conn, "CRTC_ID")?,
@@ -546,6 +572,14 @@ impl<A: AsRawFd + 'static> AtomicDrmSurfaceInternal<A> {
             );
         }
 
+        for conn in removed_connectors {
+            req.add_property(
+                *conn,
+                self.conn_prop_handle(*conn, "CRTC_ID")?,
+                property::Value::CRTC(None),
+            );
+        }
+
         if let Some(blob) = blob {
             req.add_property(self.crtc, self.crtc_prop_handle(self.crtc, "MODE_ID")?, blob);
         }
@@ -816,6 +850,10 @@ impl<A: AsRawFd + 'static> Surface for AtomicDrmSurface<A> {
         self.0.remove_connector(connector)
     }
 
+    fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Error> {
+        self.0.set_connectors(connectors)
+    }
+
     fn use_mode(&self, mode: Option<Mode>) -> Result<(), Error> {
         self.0.use_mode(mode)
     }

src/backend/drm/egl/surface.rs

@@ -49,6 +49,10 @@ where
             .map_err(Error::Underlying)
     }
 
+    fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Self::Error> {
+        self.surface.set_connectors(connectors).map_err(Error::Underlying)
+    }
+
     fn current_mode(&self) -> Option<Mode> {
         self.surface.current_mode()
     }

src/backend/drm/gbm/surface.rs

@@ -163,6 +163,10 @@ impl<D: RawDevice + 'static> Surface for GbmSurfaceInternal<D> {
         self.crtc.remove_connector(connector).map_err(Error::Underlying)
     }
 
+    fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Self::Error> {
+        self.crtc.set_connectors(connectors).map_err(Error::Underlying)
+    }
+
     fn current_mode(&self) -> Option<Mode> {
         self.crtc.current_mode()
     }
@@ -317,6 +321,10 @@ impl<D: RawDevice + 'static> Surface for GbmSurface<D> {
         self.0.remove_connector(connector)
     }
 
+    fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Self::Error> {
+        self.0.set_connectors(connectors)
+    }
+
     fn current_mode(&self) -> Option<Mode> {
         self.0.current_mode()
     }

src/backend/drm/legacy/mod.rs

@@ -233,6 +233,9 @@ impl<A: AsRawFd + 'static> Device for LegacyDrmDevice<A> {
             }
         }
 
+        // Now try to do the least possible amount of changes to set this to the state the users requested
+        // TODO!
+        
         let state = State { mode, connectors };
         let backend = Rc::new(LegacyDrmSurfaceInternal {
             dev: self.dev.clone(),

src/backend/drm/legacy/surface.rs

@@ -105,58 +105,13 @@ impl<A: AsRawFd + 'static> Surface for LegacyDrmSurfaceInternal<A> {
     }
 
     fn add_connector(&self, conn: connector::Handle) -> Result<(), Error> {
-        let info = self
-            .get_connector(conn)
-            .compat()
-            .map_err(|source| Error::Access {
-                errmsg: "Error loading connector info",
-                dev: self.dev_path(),
-                source,
-            })?;
-
         let mut pending = self.pending.write().unwrap();
 
-        // check if the connector can handle the current mode
+        if self.check_connector(conn, pending.mode.as_ref().unwrap())? {
-        if info.modes().contains(pending.mode.as_ref().unwrap()) {
-            // check if there is a valid encoder
-            let encoders = info
-                .encoders()
-                .iter()
-                .filter(|enc| enc.is_some())
-                .map(|enc| enc.unwrap())
-                .map(|encoder| {
-                    self.get_encoder(encoder)
-                        .compat()
-                        .map_err(|source| Error::Access {
-                            errmsg: "Error loading encoder info",
-                            dev: self.dev_path(),
-                            source,
-                        })
-                })
-                .collect::<Result<Vec<encoder::Info>, _>>()?;
-
-            // and if any encoder supports the selected crtc
-            let resource_handles = self.resource_handles().compat().map_err(|source| Error::Access {
-                errmsg: "Error loading resources",
-                dev: self.dev_path(),
-                source,
-            })?;
-            if !encoders
-                .iter()
-                .map(|encoder| encoder.possible_crtcs())
-                .all(|crtc_list| resource_handles.filter_crtcs(crtc_list).contains(&self.crtc))
-            {
-                return Err(Error::NoSuitableEncoder {
-                    connector: info.handle(),
-                    crtc: self.crtc,
-                });
-            }
-
             pending.connectors.insert(conn);
-            Ok(())
-        } else {
-            Err(Error::ModeNotSuitable(pending.mode.unwrap()))
         }
+
+        Ok(())
     }
 
     fn remove_connector(&self, connector: connector::Handle) -> Result<(), Error> {
@@ -164,6 +119,21 @@ impl<A: AsRawFd + 'static> Surface for LegacyDrmSurfaceInternal<A> {
         Ok(())
     }
     
+    fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Self::Error> {
+        let mut pending = self.pending.write().unwrap();
+        
+        if connectors
+            .iter()
+            .map(|conn| self.check_connector(*conn, pending.mode.as_ref().unwrap()))
+            .collect::<Result<Vec<bool>, _>>()?
+            .iter().all(|v| *v)
+        {
+            pending.connectors = connectors.into_iter().cloned().collect();
+        }
+
+        Ok(())
+    }
+
     fn use_mode(&self, mode: Option<Mode>) -> Result<(), Error> {
         let mut pending = self.pending.write().unwrap();
 
@@ -279,6 +249,57 @@ impl<A: AsRawFd + 'static> RawSurface for LegacyDrmSurfaceInternal<A> {
     }
 }
 
+impl<A: AsRawFd + 'static> LegacyDrmSurfaceInternal<A> {
+    fn check_connector(&self, conn: connector::Handle, mode: &Mode) -> Result<bool, Error> {
+        let info = self
+            .get_connector(conn)
+            .compat()
+            .map_err(|source| Error::Access {
+                errmsg: "Error loading connector info",
+                dev: self.dev_path(),
+                source,
+            })?;
+
+        // check if the connector can handle the current mode
+        if info.modes().contains(mode) {
+            // check if there is a valid encoder
+            let encoders = info
+                .encoders()
+                .iter()
+                .filter(|enc| enc.is_some())
+                .map(|enc| enc.unwrap())
+                .map(|encoder| {
+                    self.get_encoder(encoder)
+                        .compat()
+                        .map_err(|source| Error::Access {
+                            errmsg: "Error loading encoder info",
+                            dev: self.dev_path(),
+                            source,
+                        })
+                })
+                .collect::<Result<Vec<encoder::Info>, _>>()?;
+
+            // and if any encoder supports the selected crtc
+            let resource_handles = self.resource_handles().compat().map_err(|source| Error::Access {
+                errmsg: "Error loading resources",
+                dev: self.dev_path(),
+                source,
+            })?;
+            if !encoders
+                .iter()
+                .map(|encoder| encoder.possible_crtcs())
+                .all(|crtc_list| resource_handles.filter_crtcs(crtc_list).contains(&self.crtc))
+            {
+                Ok(false)
+            } else {
+                Ok(true)
+            }
+        } else {
+            Ok(false)
+        }
+    }
+}
+
 impl<A: AsRawFd + 'static> Drop for LegacyDrmSurfaceInternal<A> {
     fn drop(&mut self) {
         // ignore failure at this point
@@ -347,6 +368,10 @@ impl<A: AsRawFd + 'static> Surface for LegacyDrmSurface<A> {
         self.0.remove_connector(connector)
     }
 
+    fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Self::Error> {
+        self.0.set_connectors(connectors)
+    }
+
     fn use_mode(&self, mode: Option<Mode>) -> Result<(), Error> {
         self.0.use_mode(mode)
     }

src/backend/drm/mod.rs

@@ -172,6 +172,13 @@ pub trait Surface {
     /// Tries to mark a [`connector`](drm::control::connector)
     /// for removal on the next commit.
     fn remove_connector(&self, connector: connector::Handle) -> Result<(), Self::Error>;
+    /// Tries to replace the current connector set with the newly provided one on the next commit.
+    ///
+    /// Fails if one new `connector` is not compatible with the underlying [`crtc`](drm::control::crtc)
+    /// (e.g. no suitable [`encoder`](drm::control::encoder) may be found)
+    /// or is not compatible with the currently pending
+    /// [`Mode`](drm::control::Mode).
+    fn set_connectors(&self, connectors: &[connector::Handle]) -> Result<(), Self::Error>;
     /// Returns the currently active [`Mode`](drm::control::Mode)
     /// of the underlying [`crtc`](drm::control::crtc)
     /// if any.