metal: block screencopies behind cross-device copies

src/backends/metal/present.rs

@@ -6,6 +6,7 @@ use {
             transaction::{DrmConnectorState, DrmPlaneState},
             video::{
                 MetalConnector, MetalCrtc, MetalHardwareCursorChange, MetalPlane, RenderBuffer,
+                RenderBufferCopy,
             },
         },
         cmm::cmm_description::ColorDescription,
@@ -71,7 +72,7 @@ pub struct PresentFb {
     fb: Rc<DrmFramebuffer>,
     tex: Rc<dyn GfxTexture>,
     direct_scanout_data: Option<DirectScanoutData>,
-    sync_file: Option<SyncFile>,
+    copy: RenderBufferCopy,
     pub locked: bool,
 }
 
@@ -103,6 +104,12 @@ pub const DEFAULT_PRE_COMMIT_MARGIN: u64 = 16_000_000; // 16ms
 pub const DEFAULT_POST_COMMIT_MARGIN: u64 = 1_500_000; // 1.5ms;
 pub const POST_COMMIT_MARGIN_DELTA: u64 = 500_000; // 500us
 
+#[derive(Copy, Clone)]
+enum PresentFbWait {
+    Render,
+    Scanout,
+}
+
 impl MetalConnector {
     pub fn schedule_present(&self) {
         self.present_trigger.trigger();
@@ -227,6 +234,13 @@ impl MetalConnector {
             direct_scanout_id = fb.direct_scanout_data.as_ref().map(|d| d.dma_buf_id);
             present_fb = Some(fb);
         }
+        self.await_present_fb(present_fb.as_mut(), PresentFbWait::Render)
+            .await;
+        // perform_screencopies should return a sync file that we wait on before
+        // presentation since during screencopy the buffer layout might be mutated which
+        // could interfere with scanout. However, perform_screencopies just uses the
+        // current PresentFb if present_fb is None, potentially mutating the fb that is
+        // currently being scanned out, which would render such a wait absurd.
         self.perform_screencopies(&present_fb, &node, &cd);
         if let Some(sync_file) = self.cursor_sync_file.take()
             && let Err(e) = self.state.ring.readable(&sync_file).await
@@ -236,7 +250,8 @@ impl MetalConnector {
                 ErrorFmt(e)
             );
         }
-        self.await_present_fb(present_fb.as_mut()).await;
+        self.await_present_fb(present_fb.as_mut(), PresentFbWait::Scanout)
+            .await;
         let mut changed_planes = ArrayVec::new();
         let mut res = self.program_connector(
             version,
@@ -259,7 +274,8 @@ impl MetalConnector {
                 false,
             )?;
             present_fb = Some(fb);
-            self.await_present_fb(present_fb.as_mut()).await;
+            self.await_present_fb(present_fb.as_mut(), PresentFbWait::Scanout)
+                .await;
             res = self.program_connector(
                 version,
                 &crtc,
@@ -330,17 +346,26 @@ impl MetalConnector {
         }
     }
 
-    async fn await_present_fb(&self, new_fb: Option<&mut PresentFb>) {
+    async fn await_present_fb(&self, new_fb: Option<&mut PresentFb>, wait: PresentFbWait) {
+        use PresentFbWait as W;
         let Some(fb) = new_fb else {
             return;
         };
-        let Some(sync_file) = fb.sync_file.take() else {
+        let field = match wait {
+            W::Render => &mut fb.copy.render_block,
+            W::Scanout => &mut fb.copy.present_block,
+        };
+        let Some(sync_file) = field.take() else {
             return;
         };
         if let Err(e) = self.state.ring.readable(&sync_file).await {
+            let name = match wait {
+                W::Render => "render",
+                W::Scanout => "scanout",
+            };
             log::error!(
-                "Could not wait for primary sync file to complete: {}",
+                "Could not wait for primary {name} sync file to complete: {}",
-                ErrorFmt(e)
+                ErrorFmt(e),
             );
         }
     }
@@ -532,7 +557,7 @@ impl MetalConnector {
         let swap_buffers = c.cursor_swap_buffer.is_some();
         self.cursor_swap_buffer.set(swap_buffers);
         if let Some(sf) = c.cursor_swap_buffer.take() {
-            let sf = c.cursor_buffer.copy_to_dev(cd, sf)?;
+            let sf = c.cursor_buffer.copy_to_dev(cd, sf)?.present_block;
             self.cursor_sync_file.set(sf);
         }
         let mut cursor_changed = false;
@@ -831,7 +856,7 @@ impl MetalConnector {
             };
             log::debug!("{} direct scanout on {}", change, self.kernel_id());
         }
-        let sync_file;
+        let copy;
         let fb;
         let tex;
         match &direct_scanout_data {
@@ -848,17 +873,18 @@ impl MetalConnector {
                         blend_cd,
                     )
                     .map_err(MetalError::RenderFrame)?;
-                sync_file = buffer.copy_to_dev(cd, sf)?;
+                copy = buffer.copy_to_dev(cd, sf)?;
                 fb = buffer.drm.clone();
                 tex = buffer.render_tex.clone();
             }
             Some(dsd) => {
-                sync_file = match &dsd.acquire_sync {
+                let sf = match &dsd.acquire_sync {
                     AcquireSync::None => None,
                     AcquireSync::Implicit => None,
                     AcquireSync::SyncFile { sync_file } => Some(sync_file.clone()),
                     AcquireSync::Unnecessary => None,
                 };
+                copy = RenderBufferCopy::for_both(sf);
                 fb = dsd.fb.clone();
                 tex = dsd.tex.clone();
             }
@@ -867,7 +893,7 @@ impl MetalConnector {
             fb,
             tex,
             direct_scanout_data,
-            sync_file,
+            copy,
             locked: latched.locked,
         })
     }

src/backends/metal/video.rs

@@ -2965,6 +2965,21 @@ pub struct RenderBuffer {
     pub render_fb: Option<Rc<dyn GfxFramebuffer>>,
 }
 
+#[derive(Default)]
+pub struct RenderBufferCopy {
+    pub render_block: Option<SyncFile>,
+    pub present_block: Option<SyncFile>,
+}
+
+impl RenderBufferCopy {
+    pub fn for_both(sf: Option<SyncFile>) -> Self {
+        Self {
+            render_block: sf.clone(),
+            present_block: sf,
+        }
+    }
+}
+
 impl RenderBuffer {
     pub fn render_fb(&self) -> Rc<dyn GfxFramebuffer> {
         self.render_fb
@@ -2976,9 +2991,12 @@ impl RenderBuffer {
         &self,
         cd: &Rc<ColorDescription>,
         sync_file: Option<SyncFile>,
-    ) -> Result<Option<SyncFile>, MetalError> {
+    ) -> Result<RenderBufferCopy, MetalError> {
         let Some(tex) = &self.dev_tex else {
-            return Ok(sync_file);
+            return Ok(RenderBufferCopy {
+                render_block: None,
+                present_block: sync_file,
+            });
         };
         self.dev_fb
             .copy_texture(
@@ -2994,6 +3012,7 @@ impl RenderBuffer {
                 0,
             )
             .map_err(MetalError::CopyToOutput)
+            .map(RenderBufferCopy::for_both)
     }
 
     pub fn damage_full(&self) {