Skip to contentSkip to conversation

rust: drm: tyr: use read_poll_timeout

PATCH Superseded Deborah Brouwer 3 participants 3 messages Jan 15 – Jan 16

Subsystems (4)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 rust: drm: tyr: use read_poll_timeout

The L2 power-on sequence and soft reset in Tyr previously relied on fixed sleeps followed by a single register check, since polling helpers were not available in Rust at the time.

Now that read_poll_timeout() is available, poll the relevant registers until the hardware reports readiness or a timeout is reached.

This avoids unnecessary delays on start-up.

---
 drivers/gpu/drm/tyr/driver.rs | 18 +++++++++---------
 drivers/gpu/drm/tyr/gpu.rs    | 16 +++++++++-------
 2 files changed, 18 insertions(+), 16 deletions(-)
drivers/gpu/drm/tyr/driver.rs +9 -9
index f0da58932702..60d01e3f101b 100644
@@ -8,6 +8,7 @@
 use kernel::devres::Devres;
 use kernel::drm;
 use kernel::drm::ioctl;
+use kernel::io::poll;
 use kernel::new_mutex;
 use kernel::of;
 use kernel::platform;
@@ -67,19 +68,18 @@ unsafe impl Sync for TyrData {}
 fn issue_soft_reset(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {
     regs::GPU_CMD.write(dev, iomem, regs::GPU_CMD_SOFT_RESET)?;
 
-    // TODO: We cannot poll, as there is no support in Rust currently, so we
-    // sleep. Change this when read_poll_timeout() is implemented in Rust.
-    kernel::time::delay::fsleep(time::Delta::from_millis(100));
-
-    if regs::GPU_IRQ_RAWSTAT.read(dev, iomem)? & regs::GPU_IRQ_RAWSTAT_RESET_COMPLETED == 0 {
-        dev_err!(dev, "GPU reset failed with errno\n");
+    if let Err(e) = poll::read_poll_timeout(
+        || regs::GPU_IRQ_RAWSTAT.read(dev, iomem),
+        |status| *status & regs::GPU_IRQ_RAWSTAT_RESET_COMPLETED != 0,
+        time::Delta::from_millis(1),
+        time::Delta::from_millis(100),
+    ) {
         dev_err!(
             dev,
-            "GPU_INT_RAWSTAT is {}\n",
+            "GPU reset failed, GPU_IRQ_RAWSTAT is {}\n",
             regs::GPU_IRQ_RAWSTAT.read(dev, iomem)?
         );
-
-        return Err(EIO);
+        return Err(e);
     }
 
     Ok(())
drivers/gpu/drm/tyr/gpu.rs +9 -7
index 6c582910dd5d..2ec56f763cc6 100644
@@ -4,9 +4,10 @@
 use kernel::device::Bound;
 use kernel::device::Device;
 use kernel::devres::Devres;
+use kernel::io::poll;
 use kernel::platform;
 use kernel::prelude::*;
-use kernel::time;
+use kernel::time::Delta;
 use kernel::transmute::AsBytes;
 
 use crate::driver::IoMem;
@@ -206,13 +207,14 @@ fn from(value: u32) -> Self {
 pub(crate) fn l2_power_on(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {
     regs::L2_PWRON_LO.write(dev, iomem, 1)?;
 
-    // TODO: We cannot poll, as there is no support in Rust currently, so we
-    // sleep. Change this when read_poll_timeout() is implemented in Rust.
-    kernel::time::delay::fsleep(time::Delta::from_millis(100));
-
-    if regs::L2_READY_LO.read(dev, iomem)? != 1 {
+    if let Err(e) = poll::read_poll_timeout(
+        || regs::L2_READY_LO.read(dev, iomem),
+        |status| *status == 1,
+        Delta::from_millis(1),
+        Delta::from_millis(100),
+    ) {
         dev_err!(dev, "Failed to power on the GPU\n");
-        return Err(EIO);
+        return Err(e);
     }
 
     Ok(())
Signature · 1 line
2.52.0
maintainer Reviewed-by on v1 1/1
#1 lore raw
Quoted 11 lines · Deborah Brouwer

The L2 power-on sequence and soft reset in Tyr previously relied on fixed sleeps followed by a single register check, since polling helpers were not available in Rust at the time.

Now that read_poll_timeout() is available, poll the relevant registers until the hardware reports readiness or a timeout is reached.

This avoids unnecessary delays on start-up.

reviewer
#2 lore raw
In reply to Deborah Brouwer
Quoted 76 lines · Deborah Brouwer

The L2 power-on sequence and soft reset in Tyr previously relied on fixed sleeps followed by a single register check, since polling helpers were not available in Rust at the time.

Now that read_poll_timeout() is available, poll the relevant registers until the hardware reports readiness or a timeout is reached.

This avoids unnecessary delays on start-up.

---
 drivers/gpu/drm/tyr/driver.rs | 18 +++++++++---------
 drivers/gpu/drm/tyr/gpu.rs    | 16 +++++++++-------
 2 files changed, 18 insertions(+), 16 deletions(-)
drivers/gpu/drm/tyr/driver.rs +9 -9
index f0da58932702..60d01e3f101b 100644
@@ -8,6 +8,7 @@
 use kernel::devres::Devres;
 use kernel::drm;
 use kernel::drm::ioctl;
+use kernel::io::poll;
 use kernel::new_mutex;
 use kernel::of;
 use kernel::platform;
@@ -67,19 +68,18 @@ unsafe impl Sync for TyrData {}
 fn issue_soft_reset(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {
     regs::GPU_CMD.write(dev, iomem, regs::GPU_CMD_SOFT_RESET)?;
 
-    // TODO: We cannot poll, as there is no support in Rust currently, so we
-    // sleep. Change this when read_poll_timeout() is implemented in Rust.
-    kernel::time::delay::fsleep(time::Delta::from_millis(100));
-
-    if regs::GPU_IRQ_RAWSTAT.read(dev, iomem)? & regs::GPU_IRQ_RAWSTAT_RESET_COMPLETED == 0 {
-        dev_err!(dev, "GPU reset failed with errno\n");
+    if let Err(e) = poll::read_poll_timeout(
+        || regs::GPU_IRQ_RAWSTAT.read(dev, iomem),
+        |status| *status & regs::GPU_IRQ_RAWSTAT_RESET_COMPLETED != 0,
+        time::Delta::from_millis(1),
+        time::Delta::from_millis(100),
+    ) {
         dev_err!(
             dev,
-            "GPU_INT_RAWSTAT is {}\n",
+            "GPU reset failed, GPU_IRQ_RAWSTAT is {}\n",
             regs::GPU_IRQ_RAWSTAT.read(dev, iomem)?
         );
-
-        return Err(EIO);
+        return Err(e);
     }
 
     Ok(())
drivers/gpu/drm/tyr/gpu.rs +2 -2
index 6c582910dd5d..2ec56f763cc6 100644
@@ -4,9 +4,10 @@
 use kernel::device::Bound;
 use kernel::device::Device;
 use kernel::devres::Devres;
+use kernel::io::poll;
 use kernel::platform;
 use kernel::prelude::*;
-use kernel::time;
+use kernel::time::Delta;
 use kernel::transmute::AsBytes;
 
 use crate::driver::IoMem;
@@ -206,13 +207,14 @@ fn from(value: u32) -> Self {
 pub(crate) fn l2_power_on(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {
     regs::L2_PWRON_LO.write(dev, iomem, 1)?;
 
-    // TODO: We cannot poll, as there is no support in Rust currently, so we

drivers/gpu/drm/tyr/gpu.rs fn from(value: u32) -> Self { · line 215

Quoted 10 lines
drivers/gpu/drm/tyr/gpu.rs +6 -4
-    // sleep. Change this when read_poll_timeout() is implemented in Rust.
-    kernel::time::delay::fsleep(time::Delta::from_millis(100));
-
-    if regs::L2_READY_LO.read(dev, iomem)? != 1 {
+    if let Err(e) = poll::read_poll_timeout(
+        || regs::L2_READY_LO.read(dev, iomem),
+        |status| *status == 1,
+        Delta::from_millis(1),
+        Delta::from_millis(100),
+    ) {

This can be

    poll::read_poll_timeout(...).inspect_err(|_| dev_err!(...))?;

Best,
Gary

Quoted 6 lines
drivers/gpu/drm/tyr/gpu.rs +1 -1
         dev_err!(dev, "Failed to power on the GPU\n");
-        return Err(EIO);
+        return Err(e);
     }
 
     Ok(())