From 68141e8dffdb93b2a2f481ab8271c40fc3590d22 Mon Sep 17 00:00:00 2001 From: Emmanuel Gil Peyrot Date: Sun, 24 Sep 2023 19:46:13 +0200 Subject: [PATCH 1/3] control: Add a new HashMap for accessing properties by name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a different interface from what we were currently exposing to users, instead of iterating on properties and calling get_property() on each of them, do it once and store the results in a HashMap. This lowers the amount of get_property() calls for clients which don’t cache those results, and make the API a bit simpler. --- src/control/mod.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/control/mod.rs b/src/control/mod.rs index e60ee8e..abab0e1 100644 --- a/src/control/mod.rs +++ b/src/control/mod.rs @@ -50,6 +50,7 @@ use buffer; use super::util::*; +use std::collections::HashMap; use std::convert::TryFrom; use std::iter::Zip; use std::mem; @@ -1427,6 +1428,20 @@ pub struct PropertyValueSet { } impl PropertyValueSet { + /// Returns a HashMap mapping property names to info + pub fn as_hashmap( + &self, + device: &impl Device, + ) -> Result, SystemError> { + let mut map = HashMap::new(); + for id in self.prop_ids.iter() { + let info = device.get_property(*id)?; + let name = info.name().to_str().unwrap().to_owned(); + map.insert(name, info); + } + Ok(map) + } + /// Returns a pair representing a set of [`property::Handle`] and their raw values pub fn as_props_and_values(&self) -> (&[property::Handle], &[property::RawValue]) { (&self.prop_ids, &self.prop_vals) From 7503d13116bfe3c5f967ed7b438d5c5817774aa3 Mon Sep 17 00:00:00 2001 From: Emmanuel Gil Peyrot Date: Sun, 24 Sep 2023 19:46:21 +0200 Subject: [PATCH 2/3] atomic_modeset example: Use the new hashmap for properties This reduces the amount of DRM_IOCTL_MODE_OBJ_GETPROPERTIES ioctls called in this example from 32 to 12, and the amount of DRM_IOCTL_MODE_GETPROPERTY ioctls from 172 to 84. --- examples/atomic_modeset.rs | 64 +++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/examples/atomic_modeset.rs b/examples/atomic_modeset.rs index d01ab04..052927f 100644 --- a/examples/atomic_modeset.rs +++ b/examples/atomic_modeset.rs @@ -9,26 +9,8 @@ use drm::Device as BasicDevice; use drm::buffer::DrmFourcc; -use drm::control::ResourceHandle; use drm::control::{self, atomic, connector, crtc, property, AtomicCommitFlags}; -fn find_prop_id( - card: &Card, - handle: T, - name: &'static str, -) -> Option { - let props = card - .get_properties(handle) - .expect("Could not get props of connector"); - let (ids, _vals) = props.as_props_and_values(); - ids.iter() - .find(|&id| { - let info = card.get_property(*id).unwrap(); - info.name().to_str().map(|x| x == name).unwrap_or(false) - }) - .cloned() -} - pub fn main() { let card = Card::open_global(); @@ -124,73 +106,85 @@ pub fn main() { println!("{:#?}", db); println!("{:#?}", plane); + let con_props = card + .get_properties(con.handle()) + .expect("Could not get props of connector") + .as_hashmap(&card) + .expect("Could not get a prop from connector"); + let crtc_props = card + .get_properties(crtc.handle()) + .expect("Could not get props of crtc") + .as_hashmap(&card) + .expect("Could not get a prop from crtc"); + let plane_props = card + .get_properties(plane) + .expect("Could not get props of plane") + .as_hashmap(&card) + .expect("Could not get a prop from plane"); + let mut atomic_req = atomic::AtomicModeReq::new(); atomic_req.add_property( con.handle(), - find_prop_id(&card, con.handle(), "CRTC_ID").expect("Could not get CRTC_ID"), + con_props["CRTC_ID"].handle(), property::Value::CRTC(Some(crtc.handle())), ); let blob = card .create_property_blob(&mode) .expect("Failed to create blob"); + atomic_req.add_property(crtc.handle(), crtc_props["MODE_ID"].handle(), blob); atomic_req.add_property( crtc.handle(), - find_prop_id(&card, crtc.handle(), "MODE_ID").expect("Could not get MODE_ID"), - blob, - ); - atomic_req.add_property( - crtc.handle(), - find_prop_id(&card, crtc.handle(), "ACTIVE").expect("Could not get ACTIVE"), + crtc_props["ACTIVE"].handle(), property::Value::Boolean(true), ); atomic_req.add_property( plane, - find_prop_id(&card, plane, "FB_ID").expect("Could not get FB_ID"), + plane_props["FB_ID"].handle(), property::Value::Framebuffer(Some(fb)), ); atomic_req.add_property( plane, - find_prop_id(&card, plane, "CRTC_ID").expect("Could not get CRTC_ID"), + plane_props["CRTC_ID"].handle(), property::Value::CRTC(Some(crtc.handle())), ); atomic_req.add_property( plane, - find_prop_id(&card, plane, "SRC_X").expect("Could not get SRC_X"), + plane_props["SRC_X"].handle(), property::Value::UnsignedRange(0), ); atomic_req.add_property( plane, - find_prop_id(&card, plane, "SRC_Y").expect("Could not get SRC_Y"), + plane_props["SRC_Y"].handle(), property::Value::UnsignedRange(0), ); atomic_req.add_property( plane, - find_prop_id(&card, plane, "SRC_W").expect("Could not get SRC_W"), + plane_props["SRC_W"].handle(), property::Value::UnsignedRange((mode.size().0 as u64) << 16), ); atomic_req.add_property( plane, - find_prop_id(&card, plane, "SRC_H").expect("Could not get SRC_H"), + plane_props["SRC_H"].handle(), property::Value::UnsignedRange((mode.size().1 as u64) << 16), ); atomic_req.add_property( plane, - find_prop_id(&card, plane, "CRTC_X").expect("Could not get CRTC_X"), + plane_props["CRTC_X"].handle(), property::Value::SignedRange(0), ); atomic_req.add_property( plane, - find_prop_id(&card, plane, "CRTC_Y").expect("Could not get CRTC_Y"), + plane_props["CRTC_Y"].handle(), property::Value::SignedRange(0), ); atomic_req.add_property( plane, - find_prop_id(&card, plane, "CRTC_W").expect("Could not get CRTC_W"), + plane_props["CRTC_W"].handle(), property::Value::UnsignedRange(mode.size().0 as u64), ); atomic_req.add_property( plane, - find_prop_id(&card, plane, "CRTC_H").expect("Could not get CRTC_H"), + plane_props["CRTC_H"].handle(), property::Value::UnsignedRange(mode.size().1 as u64), ); From 81e8d1c9a91282930c8d364380b0f44fda0e4369 Mon Sep 17 00:00:00 2001 From: Emmanuel Gil Peyrot Date: Sun, 24 Sep 2023 20:10:44 +0200 Subject: [PATCH 3/3] drm-ffi: Call GETPROPERTY ioctl only once if no values or enums When we call this ioctl first, we obtain the amount of values and enum blobs available for this property, so that we can allocate the memory and pass it in a second call. But when there are zero values and zero enum blobs, there is no need for any allocation or second call. This further reduces the amount of DRM_IOCTL_MODE_GETPROPERTY ioctls from 84 to 77 in the atomic_modeset example. --- drm-ffi/src/mode.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/drm-ffi/src/mode.rs b/drm-ffi/src/mode.rs index 2da41fc..c71efd0 100644 --- a/drm-ffi/src/mode.rs +++ b/drm-ffi/src/mode.rs @@ -554,26 +554,25 @@ pub fn get_property( mut values: Option<&mut Vec>, mut enums: Option<&mut Vec>, ) -> Result { - let mut sizes = drm_mode_get_property { + let mut prop = drm_mode_get_property { prop_id, ..Default::default() }; unsafe { - ioctl::mode::get_property(fd, &mut sizes)?; + ioctl::mode::get_property(fd, &mut prop)?; } - map_reserve!(values, sizes.count_values as usize); - map_reserve!(enums, sizes.count_enum_blobs as usize); + // There is no need to call get_property() twice if there is nothing else to retrieve. + if prop.count_values == 0 && prop.count_enum_blobs == 0 { + return Ok(prop); + } - let mut prop = drm_mode_get_property { - prop_id, - values_ptr: map_ptr!(&values), - enum_blob_ptr: map_ptr!(&enums), - count_values: map_len!(&values), - count_enum_blobs: map_len!(&enums), - ..Default::default() - }; + map_reserve!(values, prop.count_values as usize); + map_reserve!(enums, prop.count_enum_blobs as usize); + + prop.values_ptr = map_ptr!(&values); + prop.enum_blob_ptr = map_ptr!(&enums); unsafe { ioctl::mode::get_property(fd, &mut prop)?;