Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <75366bfac9fcd4f8c35309193705f0277a164ae4.camel@nxp.com>
Subject: Re: [PATCH v2] PM: runtime: Return properly from rpm_resume() if
 dev->power.needs_force_resume flag is set
From:   Liu Ying <victor.liu@nxp.com>
To:     Ulf Hansson <ulf.hansson@linaro.org>
Cc:     linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org, linux-imx@nxp.com,
        "Rafael J . Wysocki" <rafael@kernel.org>,
        Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date:   Fri, 23 Sep 2022 23:22:35 +0800
In-Reply-To: <CAPDyKFqdHX=o4V4K8GdCr4wQ5sjr=JMG6CFAy1849=CtfoSgRQ@mail.gmail.com>
References: <20220923124904.1373936-1-victor.liu@nxp.com>
         <CAPDyKFqdHX=o4V4K8GdCr4wQ5sjr=JMG6CFAy1849=CtfoSgRQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SE5rM2VLNFlLQUVBd1JFUkswa2xNd25vNGRwajJSelR2dTA5NHVDcVdWRnlR?=
 =?utf-8?B?VXJ4a0R5Uyt4SndsbGMzQXdFMVd0T0Z1OHFUaGR2RHY1UzMxNlB2WkxKUnF0?=
 =?utf-8?B?L0RWU3BldmtveVhOSDhMWURlVjkxVXVJbi9tNjZ0UXYzeGM1ZmN3VUFVbVYr?=
 =?utf-8?B?V3MyazhnMXpjRTFFSWFSY3h1N051Zis3dnZ3Vmp5VTFTYTh2aDhkYkJZaU54?=
 =?utf-8?B?NFc0QmZsM21CdTJKTThuZkFRQTBPVWw4SGw5U1A5d0JBcnhCbzdETnhHTVFE?=
 =?utf-8?B?U1N6MU82YlRKRHhLZEU4MVBtbmg3eG1ZZDVZRlNnV0xlVXBUTmhkeDNRVnZB?=
 =?utf-8?B?anllOEJnY2lqeU9DOGd2WDMwajFRSThHaCtYTXpnT3IwemJKdTd4UjJPMmkv?=
 =?utf-8?B?N0N4N2QwT1IzcGFNZUJyMFNKSk5RVU9sWWZPd3YyRVg4YzJzb2VYd3kzNjFt?=
 =?utf-8?B?Z2NSaE9DR0RrRlAxTS8vR3owbUlvQS9jc1d4N3ZaNExtZjdJaE5MZUhNMFAv?=
 =?utf-8?B?T0Y3NkdpdDdDYjhUL0tLbTVyNHMwR3lhKzBGak4vTmtQRTVHRTZZNlNIRXFI?=
 =?utf-8?B?cS9sbmdaUTY0cVN0TjJGbHp4T1dzVzdhVWplSGdFY2FIVmR2eW0wc1dyUVZQ?=
 =?utf-8?B?M0NwdFp1NlNkdG1XdlJhUm9PR0kzWms2RDZJNkd1MmkwV1JxK2FsWnNHaEli?=
 =?utf-8?B?Sit1amw3am9FRFpTVDc4MFd5WFBaZ25LRnFSZmZ4V0RTRE5ZV3Y0VzlUdDZH?=
 =?utf-8?B?NEJJZmJsSk1YS1ZITFVoK25ycWVwOElzM0x6MGh4OUFKU3o0c0tsSXgzcUla?=
 =?utf-8?B?V3hkSkl6R0lsamFabW5EemRselNNZ3lBeXRDQmE0dWlQd3NuWGxNZGtuSnpi?=
 =?utf-8?B?ZGFmUVNqU1poNEZFbDRLRWJySUo3R3UyWkJOWGhMS2E0eWt2Ri9CdFAyVmsy?=
 =?utf-8?B?cFFXMFZjS1Q1ZUhOdytDaDNoNE43YXdkRnBVVjkwU3RmdUN6SGJNV3RXZGU4?=
 =?utf-8?B?RXhySU8yM056a3R1MnJaTlBmcmhORG5WOUJuQ3FGVXVsa0Zic080eG9Kekl0?=
 =?utf-8?B?bVhoZC9JN21OdDVEK1lWVEZRemJOWnQzU0Jtc3pQMnhTVThrMHFDSXlLTzM4?=
 =?utf-8?B?Vy9pSHNHYVZ1TmozMXBob0huc1J3WUMrNTNuN3dxM2YvVG9RMitJcGF4QWs0?=
 =?utf-8?B?TzhXQzdlOEFpUmNUdUIwNG8wYktVeDFaYTJRcENNWEtFRC91OHdoU0ZkNVhw?=
 =?utf-8?B?UWg3SlRJRTd4Q0xJZEpOOE1YTHBJcVFYem5RdERNS2RkYWFRWS9KQlUraE1I?=
 =?utf-8?B?UEVNakZ2eWExeFVkOHNVeEpPMXQ2QUJqcFRWWmF4amxnUWh1VnVWcVJVaW5W?=
 =?utf-8?B?VmRxMjhEdG5jUmRGOE0rdnBrNzY2Z1Vudk8wdFhVdVl2eXVHbnhmS2lONmhK?=
 =?utf-8?B?VjE2aTc2dzdXN1BUQVB3QkVnbVdHb2g5bEZLaEg3bDZLbmpaSFFCRjBhSzJT?=
 =?utf-8?B?aGxaWWh0ZDVsQzE0am5DWSt5ckNmMmRVdm44bFEydDRxaW5DOHBHN3VkM09R?=
 =?utf-8?B?SU1ZNXFhczYzZmdOaEEyUUs3SmxreUlGRERYNm5XMS96S2pKZ0UxUW5adDhr?=
 =?utf-8?B?UE9aaWpvQ2JLdlVXS25TU3BPaHUyZXZzblpwbnlsTmV2ZnB3KzVjTGtWTUhQ?=
 =?utf-8?B?L3R3M1Z5YVVsanRSWENTSngyUlVQQmFjVE1xMFg0WU9hWVlKOUM5eW8zdnF3?=
 =?utf-8?B?U3pDdmREMy9GUkJPenU5MEM2YzhXOHdOOURMK3JLMTl3MlhqM3BCVWplMUF4?=
 =?utf-8?B?aS9ya3Y2M0RVV29QK1UzS05ibWMvRmJ1TU9UQmJvT0JONXM5S1lPQ2Y1a1p5?=
 =?utf-8?B?WmRBMkFFd3F5VzdjamsrbWg1R1Z0S1p0M3R4TkVGQjdlVElSTDJFdmJCZCsz?=
 =?utf-8?B?NnJNS2cvcWE4bGxuemJmMUY0L3NMeVBRN3VoYzZERDBUWEdpMmY5RWJLZFVr?=
 =?utf-8?B?bjYyTzVobWhiOU0zSTBMNjE1M3lUSDBPQWxuWm5TSmZ4N2RyRCtmdVVYNUtx?=
 =?utf-8?B?UFFTUnZJV3lSdlIzV3IzU2VmYkp1WVh5cUdKUElnTU5zTTNyMTVVN2R1Wkkx?=
 =?utf-8?Q?omuf7jio8XX12eB4kr/hJD0wb?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: dca0996c-8475-4802-8d71-08da9d777ef5
X-MS-Exchange-CrossTenant-AuthSource: AM7PR04MB7046.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Sep 2022 15:22:58.4755
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: NJ8f2rVp3TwbiqnZBf7Hudr4s4BgCwVpxAx3PST2CZrHuZ4Bd4szU2csegAH1M292oIBB221nim3O8aZvmGr4w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM7PR04MB6949
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, 2022-09-23 at 15:48 +0200, Ulf Hansson wrote:
> On Fri, 23 Sept 2022 at 14:47, Liu Ying <victor.liu@nxp.com> wrote:
> > 
> > After a device transitions to sleep state through it's system
> > suspend
> > callback pm_runtime_force_suspend(), the device's driver may still
> > try
> > to do runtime PM for the device(runtime suspend first and then
> > runtime
> > resume) although runtime PM is disabled by that callback.  The
> > runtime
> > PM operations would not touch the device effectively and the device
> > is
> > assumed to be resumed through it's system resume callback
> > pm_runtime_force_resume().
> 
> This sounds like a fragile use case to me. In principle you want to
> allow the device to be runtime resumed/suspended, after the device
> has
> already been put into a low power state through the regular system
> suspend callback. Normally it seems better to prevent this from
> happening, completely.

Not sure if we really may prevent this from happening completely.

> 
> That said, in this case, I wonder if a better option would be to
> point
> ->suspend_late() to pm_runtime_force_suspend() and ->resume_early()
> to
> pm_runtime_force_resume(), rather than using the regular
> ->suspend|resume() callbacks. This should avoid the problem, I think,
> no?

I thought about this and it actually works for my particular
panel-simple case.  What worries me is that the device(DRM device in my
case) which triggers the runtime PM operations may also use            
->suspend_late/resume_early() callbacks for whatever reasons, hence no
fixed order to suspend/resume the two devices(like panel device and DRM
device).

Also, not sure if there is any sequence issue by using the            
->suspend_late/resume_early() callbacks in the panel-simple driver,
since it's written for quite a few display panels which may work with
various DRM devices - don't want to break any of them.

Regards,
Liu Ying

> 
> Note that, the PM core also disables runtime PM for the device in
> __device_suspend_late(). For good reasons.
> 
> [...]
> 
> Kind regards
> Uffe