From d17a4ead683855cfde1c6b718ae31959830a46d0 Mon Sep 17 00:00:00 2001 From: Victoria Brekenfeld Date: Fri, 19 Dec 2025 19:00:20 +0100 Subject: [PATCH] kms/surface: Simpify surface feedback creation --- src/backend/kms/mod.rs | 76 +++++++++++++------------- src/backend/kms/surface/mod.rs | 97 ++++++++++++++++++---------------- src/shell/element/surface.rs | 10 +++- src/state.rs | 29 +++++++--- 4 files changed, 121 insertions(+), 91 deletions(-) diff --git a/src/backend/kms/mod.rs b/src/backend/kms/mod.rs index 3a5933fa..3737901b 100644 --- a/src/backend/kms/mod.rs +++ b/src/backend/kms/mod.rs @@ -852,35 +852,36 @@ impl KmsGuard<'_> { if !test_only { if !surface.is_active() { + let mut planes = drm + .device() + .planes(crtc) + .with_context(|| "Failed to enumerate planes")?; + + let driver = drm.device().get_driver().ok(); + + // QUIRK: Using an overlay plane on a nvidia card breaks the display controller (wtf...) + if driver.as_ref().is_some_and(|driver| { + driver + .name() + .to_string_lossy() + .to_lowercase() + .contains("nvidia") + }) { + planes.overlay = vec![]; + } + // QUIRK: Cursor planes on evdi sometimes don't disappear correctly. + // TODO: Debug and figure out, as they can be a nice improvement. + if driver.as_ref().is_some_and(|driver| { + driver + .name() + .to_string_lossy() + .to_lowercase() + .contains("evdi") + }) { + planes.cursor = vec![]; + } + let compositor: GbmDrmOutput = { - let mut planes = drm - .device() - .planes(crtc) - .with_context(|| "Failed to enumerate planes")?; - let driver = drm.device().get_driver().ok(); - - // QUIRK: Using an overlay plane on a nvidia card breaks the display controller (wtf...) - if driver.as_ref().is_some_and(|driver| { - driver - .name() - .to_string_lossy() - .to_lowercase() - .contains("nvidia") - }) { - planes.overlay = vec![]; - } - // QUIRK: Cursor planes on evdi sometimes don't disappear correctly. - // TODO: Debug and figure out, as they can be a nice improvement. - if driver.as_ref().is_some_and(|driver| { - driver - .name() - .to_string_lossy() - .to_lowercase() - .contains("evdi") - }) { - planes.cursor = vec![]; - } - let mut renderer = self .api .single_renderer(&device.inner.render_node) @@ -908,7 +909,7 @@ impl KmsGuard<'_> { *mode, &[conn], &surface.output, - Some(planes), + Some(planes.clone()), &mut renderer, &elements, ) @@ -943,16 +944,17 @@ impl KmsGuard<'_> { .unwrap(), ) .ok(); + + let primary_formats = compositor_ref.surface().plane_info().formats.clone(); + let overlay_formats = planes + .overlay + .iter() + .flat_map(|p| p.formats.iter().cloned()) + .collect::(); surface.resume( compositor, - compositor_ref.surface().plane_info().formats.clone(), - compositor_ref - .surface() - .planes() - .overlay - .iter() - .flat_map(|p| p.formats.iter().cloned()) - .collect::(), + primary_formats, + Some(overlay_formats).filter(|f| !f.indexset().is_empty()), ); surface.output.set_adaptive_sync_support(vrr_support); diff --git a/src/backend/kms/surface/mod.rs b/src/backend/kms/surface/mod.rs index 18e291d1..46865bd3 100644 --- a/src/backend/kms/surface/mod.rs +++ b/src/backend/kms/surface/mod.rs @@ -120,7 +120,7 @@ pub struct Surface { active: Arc, pub(super) feedback: HashMap, pub(super) primary_plane_formats: FormatSet, - overlay_plane_formats: FormatSet, + overlay_plane_formats: Option, loop_handle: LoopHandle<'static, State>, thread_command: Sender, @@ -343,7 +343,7 @@ impl Surface { active, feedback: HashMap::new(), primary_plane_formats: FormatSet::default(), - overlay_plane_formats: FormatSet::default(), + overlay_plane_formats: None, loop_handle: evlh.clone(), thread_command: tx, thread_token, @@ -436,7 +436,7 @@ impl Surface { &mut self, compositor: GbmDrmOutput, primary_plane_formats: FormatSet, - overlay_plane_formats: FormatSet, + overlay_plane_formats: Option, ) { self.primary_plane_formats = primary_plane_formats; self.overlay_plane_formats = overlay_plane_formats; @@ -1528,75 +1528,82 @@ fn get_surface_dmabuf_feedback( render_node: DrmNode, target_node: DrmNode, render_formats: FormatSet, - target_formats: FormatSet, + _target_formats: FormatSet, primary_plane_formats: FormatSet, - overlay_plane_formats: FormatSet, + overlay_plane_formats: Option, ) -> SurfaceDmabufFeedback { - let combined_formats = render_formats - .intersection(&target_formats) - .copied() - .collect::(); - // We limit the scan-out trache to formats we can also render from // so that there is always a fallback render path available in case // the supplied buffer can not be scanned out directly - let primary_plane_formats = primary_plane_formats - .intersection(&combined_formats) - .copied() - .collect::(); - let overlay_plane_formats = overlay_plane_formats - .intersection(&combined_formats) - .copied() - .collect::(); + let primary_plane_formats = primary_plane_formats + .intersection(&render_formats) + .cloned() + .collect::(); + let overlay_plane_formats = overlay_plane_formats.map(|formats| { + formats + .intersection(&render_formats) + .cloned() + .collect::() + }); let builder = DmabufFeedbackBuilder::new(render_node.dev_id(), render_formats); + /* - // iris doesn't handle nvidia buffers very well (it hangs). - // so only do this in the future with v6 and clients telling us the gpu + // Sadly no implementation would pick this up as a preferred render tranche, + // where the combined formats would increase our chances of doing a dmabuf copy. + // .. So we should probably not advertise this on the off-chance it actually triggers bugs. + // + + let combined_formats = render_formats.intersection(&target_formats).cloned().collect::(); if target_node != render_node.dev_id() && !combined_formats.is_empty() { builder = builder.add_preference_tranche( - target_node, + render_node.dev_id(), + None, + combined_formats, + ); + }; + + // We also can't advertise scan out tranches for the actual display device, + // as e.g. the nvidia driver might then send us dmabufs, that makes e.g. the iris hangs on import... + if target_node != render_node.dev_id() && !combined_formats.is_empty() { + builder = builder.add_preference_tranche( + target_node.dev_id(), Some(zwp_linux_dmabuf_feedback_v1::TrancheFlags::Scanout), combined_formats, ); }; + + // So no fun combinations, we gotta wait for dmabuf-v6 */ let render_feedback = builder.clone().build().unwrap(); - // we would want to do this in other cases as well, but same thing as above applies - let primary_scanout_feedback = if target_node == render_node { + let primary_scanout_feedback = (target_node == render_node).then(|| { builder .clone() .add_preference_tranche( - target_node.dev_id(), + render_node.dev_id(), Some(zwp_linux_dmabuf_feedback_v1::TrancheFlags::Scanout), - primary_plane_formats.clone(), + primary_plane_formats, ) .build() .unwrap() - } else { - builder.clone().build().unwrap() - }; - let scanout_feedback = if target_node == render_node { - builder - .add_preference_tranche( - target_node.dev_id(), - Some(zwp_linux_dmabuf_feedback_v1::TrancheFlags::Scanout), - FormatSet::from_iter( - primary_plane_formats - .into_iter() - .chain(overlay_plane_formats), - ), - ) - .build() - .unwrap() - } else { - builder.build().unwrap() - }; + }); + let overlay_scanout_feedback = overlay_plane_formats + .filter(|_| target_node == render_node) + .map(|formats| { + builder + .add_preference_tranche( + render_node.dev_id(), + Some(zwp_linux_dmabuf_feedback_v1::TrancheFlags::Scanout), + formats, + ) + .build() + .unwrap() + }); SurfaceDmabufFeedback { render_feedback, - scanout_feedback, + overlay_scanout_feedback, primary_scanout_feedback, } } diff --git a/src/shell/element/surface.rs b/src/shell/element/surface.rs index 93f9ab0b..9154835c 100644 --- a/src/shell/element/surface.rs +++ b/src/shell/element/surface.rs @@ -688,9 +688,15 @@ impl CosmicSurface { render_element_states, &feedback.render_feedback, if is_fullscreen { - &feedback.primary_scanout_feedback + feedback + .primary_scanout_feedback + .as_ref() + .unwrap_or(&feedback.render_feedback) } else { - &feedback.scanout_feedback + feedback + .overlay_scanout_feedback + .as_ref() + .unwrap_or(&feedback.render_feedback) }, ) }) diff --git a/src/state.rs b/src/state.rs index 7da07209..6d1c3217 100644 --- a/src/state.rs +++ b/src/state.rs @@ -313,8 +313,8 @@ pub enum LockedBackend<'a> { #[derive(Debug, Clone)] pub struct SurfaceDmabufFeedback { pub render_feedback: DmabufFeedback, - pub scanout_feedback: DmabufFeedback, - pub primary_scanout_feedback: DmabufFeedback, + pub overlay_scanout_feedback: Option, + pub primary_scanout_feedback: Option, } #[derive(Debug)] @@ -1035,7 +1035,10 @@ impl Common { surface, render_element_states, &feedback.render_feedback, - &feedback.primary_scanout_feedback, + feedback + .primary_scanout_feedback + .as_ref() + .unwrap_or(&feedback.render_feedback), ) }, ) @@ -1064,7 +1067,10 @@ impl Common { surface, render_element_states, &feedback.render_feedback, - &feedback.scanout_feedback, + feedback + .overlay_scanout_feedback + .as_ref() + .unwrap_or(&feedback.render_feedback), ) }, ); @@ -1085,7 +1091,10 @@ impl Common { surface, render_element_states, &feedback.render_feedback, - &feedback.scanout_feedback, + feedback + .overlay_scanout_feedback + .as_ref() + .unwrap_or(&feedback.render_feedback), ) }, ); @@ -1192,7 +1201,10 @@ impl Common { surface, render_element_states, &feedback.render_feedback, - &feedback.scanout_feedback, + feedback + .overlay_scanout_feedback + .as_ref() + .unwrap_or(&feedback.render_feedback), ) }, ) @@ -1214,7 +1226,10 @@ impl Common { surface, render_element_states, &feedback.render_feedback, - &feedback.scanout_feedback, + feedback + .overlay_scanout_feedback + .as_ref() + .unwrap_or(&feedback.render_feedback), ) }, );