almindor / mipidsi

MIPI Display Serial Interface unified driver
MIT License
117 stars 46 forks source link

feat: Added sleep/wake methods to display #75

Closed xgroleau closed 11 months ago

xgroleau commented 12 months ago

I've added the sleep and wake method to the Display struct. Note that I have not bothered with idle since it only allows 8bit colors, but we can add it if you prefer.

It saves about 20mW (5mA at 4V) to just put the display to sleep when the display is off.

Here is the reference of what the different settings of the modes do (for the st7789 anyway)

1. Normal Mode On (full display), Idle Mode Off, Sleep Out.
In this mode, the display is able to show maximum 262,144 colors.

2. Partial Mode On, Idle Mode Off, Sleep Out.
In this mode part of the display is used with maximum 262,144 colors.

3. Normal Mode On (full display), Idle Mode On, Sleep Out.
In this mode, the full display area is used but with 8 colors.

4. Partial Mode On, Idle Mode On, Sleep Out.
In this mode, part of the display is used but with 8 colors.

5. Sleep In Mode
In this mode, the DC: DC converter, internal oscillator and panel driver circuit are stopped. Only the MCU

Pretty simple PR since it was easy to add to the codebase but feel free to add any suggestions

rfuest commented 12 months ago

Some comments:

almindor commented 12 months ago

Some comments:

  • If we don't use a wrapper type or typestates to track the sleep state we should have a sleep_state() -> SleepState (or similar) getter to allow the user to get the current sleep state.
  • There are some timing requirements when entering/exiting the sleep mode, how do we make sure these aren't violated?
  • Not all commands are valid in sleep mode. I'm not sure if we use any commands that aren't valid in sleep mode at the moment, but if we do they should return a special error to notify the user that the operation isn't supported in sleep mode.

We should def. have a state getter, but it means an additional variable on the display level.

All good concerns. When it comes to timing we'll probably have to give the driver a &mut DelayUs to the command calls. The timings can probably be unified accross models.

AFAIK we don't use invalid commands in sleep mode, since the only commands outside of init we use are windowing/drawing.

xgroleau commented 12 months ago

All good concerns. When it comes to timing we'll probably have to give the driver a &mut DelayUs to the command calls. The timings can probably be unified accross models.

I could see a method to the DcsCommand trait that where we would pass the &mut DelayUs. We could then specify the delay in the dcs_basic_command macro and the DCS would handle the delay.

Ex:

pub trait DcsCommand {
    /// Returns the instruction code.
    fn instruction(&self) -> u8;

    /// Fills the given buffer with the command parameters.
    fn fill_params_buf(&self, buffer: &mut [u8]) -> Result<usize, Error>;

    /// Delay to wait after the command
    fn post_command_delay<D: DelayUs>(&self, delay: &mut D); // I think DelayUs is now infaillable, so we might not need a result, or we return the number of ms
    /// Or instead use a const
    const POST_COMMAND_DELAY: u32;
}

Then if we go this way, should the Display or DCS struct take ownership of the DelayUs?

rfuest commented 11 months ago

I could see a method to the DcsCommand trait that where we would pass the &mut DelayUs

DcsCommand isn't the correct location to add a delay. DcsCommand is only used to represent DCS commands and convert them to the corresponding byte sequence. It doesn't do any communication or handle the higher level requirements a command might have, like delays.

Then if we go this way, should the Display or DCS struct take ownership of the DelayUs?

At the moment it should be enough if the sleep and wake methods take a borrowed DelayUs. To reduce unnecessary delays we would need to be able to measure the time between the wake call and the previous sleep call, which I don't think is possible with just embedded_hal. So we will need to assume the worst case and add pretty long delays to sleep and wake instead.

xgroleau commented 11 months ago

From the doc, all the currently supported display requires a 120ms delay after sleep. For wake, the ST line requires the highest wait before issuing commands with 120ms.

Though the user could implement another Model himself in his user code since it's public. The timing might be different but there's no wait to control that really.

rfuest commented 11 months ago

Didn't we decide to add a sleep state getter?

almindor commented 11 months ago

Didn't we decide to add a sleep state getter?

:man_facepalming:

Sorry, I'll add an issue for it so I don't forget to add it.

xgroleau commented 11 months ago

My bad, totally forgot 😅