Closed
Bug 890541
Opened 11 years ago
Closed 11 years ago
(jb-gonk) Call power_module_t::setInteractive() when display is enabled
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: m1, Assigned: tkundu)
References
Details
(Whiteboard: [fixed-in-birch])
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
mwu
:
review+
|
Details | Diff | Splinter Review |
We should mimic the Android framework and call setInteractive(true) when the display is enabled, and setInteractive(false) when not. Ref: https://www.codeaurora.org/cgit/quic/la/platform/hardware/libhardware/tree/include/hardware/power.h?h=jb_mr1#n92
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Comment 2•11 years ago
|
||
This patch loads android jb power hal . It uses ib's powerhal->setinteractive function to disable/enable Display. It performs same sequence which android does here https://github.com/android/platform_frameworks_base/blob/master/services/java/com/android/server/power/PowerManagerService.java#L2576
Attachment #771693 -
Attachment is obsolete: true
Attachment #771707 -
Flags: review?(mwu)
Comment 3•11 years ago
|
||
Comment on attachment 771707 [details] [diff] [review] (gonk-jb) Call setInteractive() when display is enabled/disable Please rebase your patch against a newer gecko.
Attachment #771707 -
Flags: review?(mwu)
Assignee | ||
Comment 4•11 years ago
|
||
I will update another patch here soon. thanks
Assignee | ||
Comment 5•11 years ago
|
||
new patch . Please review it.
Attachment #771707 -
Attachment is obsolete: true
Attachment #772374 -
Flags: review?(mwu)
Comment 6•11 years ago
|
||
Comment on attachment 772374 [details] [diff] [review] (gonk-jb) Call setInteractive() when display is enabled/disable Review of attachment 772374 [details] [diff] [review]: ----------------------------------------------------------------- Thanks - looks fine overall. There are some nits I'd like to have fixed. Also, your patch has windows line terminators - please use unix line endings. ::: widget/gonk/libdisplay/GonkDisplayJB.cpp @@ +127,2 @@ > autosuspend_disable(); > + if (mPowerModule) { mPowerModule is not optional, so don't check for it. We should crash instead if there is no power module. @@ +142,2 @@ > autosuspend_enable(); > + if (mPowerModule) { Ditto ::: widget/gonk/libdisplay/GonkDisplayJB.h @@ +18,5 @@ > > #include "GonkDisplay.h" > #include "FramebufferSurface.h" > #include "hardware/hwcomposer.h" > +#include <hardware/power.h> The includes in this file use "" instead of <>. @@ +49,5 @@ > hw_module_t const* mModule; > hw_module_t const* mFBModule; > hwc_composer_device_1_t* mHwc; > framebuffer_device_t* mFBDevice; > + power_module_t* mPowerModule; Align the variable name with the other variables.
Assignee | ||
Comment 7•11 years ago
|
||
fixed all nits. Please review it.
Attachment #772374 -
Attachment is obsolete: true
Attachment #772374 -
Flags: review?(mwu)
Attachment #772793 -
Flags: review?(mwu)
Assignee | ||
Updated•11 years ago
|
Status: NEW → ASSIGNED
Comment 8•11 years ago
|
||
Comment on attachment 772793 [details] [diff] [review] (gonk-jb) Call setInteractive() when display is enabled/disable Review of attachment 772793 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks. BTW whatever you're using to generate patches is still generating windows line endings.
Attachment #772793 -
Flags: review?(mwu) → review+
Assignee | ||
Updated•11 years ago
|
Whiteboard: checkin-needed
Reporter | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/projects/birch/rev/0a7c9fbe5c45
Whiteboard: checkin-needed → [fixed-in-birch]
Comment 10•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0a7c9fbe5c45
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•