(jb-gonk) Call power_module_t::setInteractive() when display is enabled

RESOLVED FIXED

Status

Firefox OS
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: m1, Assigned: tkundu)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-in-birch])

Attachments

(1 attachment, 3 obsolete attachments)

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
Created attachment 771693 [details]
(gonk-jb) Call setInteractive() when display is enabled/disable
Created attachment 771707 [details] [diff] [review]
(gonk-jb) Call setInteractive() when display is enabled/disable

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

4 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)
I will update another patch here soon. thanks
Created attachment 772374 [details] [diff] [review]
(gonk-jb) Call setInteractive() when display is enabled/disable

new patch . Please review it.
Attachment #771707 - Attachment is obsolete: true
Attachment #772374 - Flags: review?(mwu)

Comment 6

4 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.
Created attachment 772793 [details] [diff] [review]
(gonk-jb) Call setInteractive() when display is enabled/disable

fixed all nits. Please review it.
Attachment #772374 - Attachment is obsolete: true
Attachment #772374 - Flags: review?(mwu)
Attachment #772793 - Flags: review?(mwu)
Status: NEW → ASSIGNED

Comment 8

4 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+
Whiteboard: checkin-needed
(Reporter)

Comment 9

4 years ago
https://hg.mozilla.org/projects/birch/rev/0a7c9fbe5c45
Whiteboard: checkin-needed → [fixed-in-birch]
https://hg.mozilla.org/mozilla-central/rev/0a7c9fbe5c45
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.