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: <9ac021dcf3c6a7a3dfa3d88dfcde18fac8a74243.camel@nxp.com>
Subject: Re: [PATCH RESEND] drm/self_refresh: Don't trigger the entry timer
 for self refresh work if CRTC state is inactive
From:   Liu Ying <victor.liu@nxp.com>
To:     dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Cc:     linux-imx@nxp.com, Rob Clark <robdclark@chromium.org>,
        Sean Paul <seanpaul@chromium.org>,
        Zain Wang <wzz@rock-chips.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>
Date:   Mon, 07 Mar 2022 10:33:41 +0800
In-Reply-To: <20220207054617.987811-1-victor.liu@nxp.com>
References: <20220207054617.987811-1-victor.liu@nxp.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.4-0ubuntu1 
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Q2l6bEZtMDJmL05LUUlsQ1ArTXVwOTJRRy9WSVJDeE9jMGRMQlNUQUFKdU1z?=
 =?utf-8?B?VDY5RzhZRVVxUFY1N3VJWkJ0dXUxcDg4ZlZvbmNJMkNBQ0ttQWhLazN1U1I3?=
 =?utf-8?B?WW1vUVRUeENFWHBETW5Qak9Oc2d4OUNITFQ2TUs5QnlncSt6c09hZFN5bkhq?=
 =?utf-8?B?bHMrU3JhOFgxTXdrRy9vWVBYZlBWMGsrZWd0eTF3TTBhSEdXM2UyRWlqQTRM?=
 =?utf-8?B?eG5mOVBZL0tCcTBEWmZRV2grUUZjMDVsdm4ybUhSdmlwcVZMZW5ZVVpkVVhD?=
 =?utf-8?B?YlZNdC9iemlvVFBVQVhMcHZyeVNSVnZlUW00VExQeHEzbmdXNktlbWxwbk5o?=
 =?utf-8?B?Y05EZmRsdnpseXpuU1lUNDhPbUJhK0NoRzNVVUU0TVpKRDlFNFdtSDZVYkhG?=
 =?utf-8?B?WStsa2VFM0djR05mZmc5bXZCSDNxTWtLdGU2SEI5ckFDcUpXY2haelAzaUdl?=
 =?utf-8?B?TXZCSkNvbXRacGpYenhTTVYzS21KUzA0eTh2YU1OZTdsZ0ZCR3E2WlF6VFhl?=
 =?utf-8?B?cmxIWThreXo0S3VVUDlZRTRqOFJCaW45Zm1DSkhhb2NBS3NNdnZucjI4a0FC?=
 =?utf-8?B?T25UbDFQR1hJNDVoSUFyb3cwRW5MS3JtYXM4TlVCNURESlU5NWlkVEZoQmJI?=
 =?utf-8?B?eDN5YVN1WHpIMGVvQjNKbXBMOFRIUTJoZ0w1WTZnUFVGUXpWUEZ4MDU2SWRh?=
 =?utf-8?B?TmpLdkI4VXlaZloyMEU5WDJMdmRwMU1hRko3Zk5wSjBpOTJYcHE5RFJPRDRS?=
 =?utf-8?B?TDlBY1ZMaVhTKzgwejI0ZWp1K3pjeEpTUWxQVnVyZnF6WVdQZWE3WEErQmVy?=
 =?utf-8?B?blpwOVpaSHFTWjJ0UVhtMTlXYWZJYzE5aXBxcTZuYUU1SElSd3Vmc2JoWWh2?=
 =?utf-8?B?UHR6ZklRanl5ZVFWZnJPSktyMlpwRDZFdStGWFNXb0lvQk1ZWERQV0tEaUk1?=
 =?utf-8?B?OUd2cXR3V1dXL2xFZWNYVmxPYXNTM1RnR0tzTEs5cUM0OVhmc0lWMHh0MUk1?=
 =?utf-8?B?VkNQdmZxNDVvcDRxUS83TVpqb052NElBM1ByemZyeEh5eXB4VWVVR3ZvbnNL?=
 =?utf-8?B?TUhranovOTdBM094SEo5V0V6R0VlVTVxUmpPZldyS0RsOVc3VnkzWDhiWjMv?=
 =?utf-8?B?QUJhblNnRGhWODQ3R1VMOXRMaTlHVE9tRFZJcGJVNDVyVlBjeUVnb1o4WUxD?=
 =?utf-8?B?QlR0V0QzLzNsNlpnUDRPVjAyRkJtSFlqdHZhb2RJTklJbXlBTzByUzlnQWNU?=
 =?utf-8?B?eURwQmhOUGxIZGRLS3l0dVZqOGxIVjRlYTRnWnRqQWVtRzhrUURlRk05MjV0?=
 =?utf-8?B?R3RmSTVlTzZsVWw3QUZVT3IxSGp2N2dvUW42RStKMEQzc1o4UGR6OWpLWHpT?=
 =?utf-8?B?WGs3SFd1T0FGQldKQnc4OFV6QzdVdG5rdjl4M1NXelVIenNVYUNKZTBIeWUy?=
 =?utf-8?B?d1MySVl6OXBzM0hGR29qbEZnQ0VMQktYanJORGhZU2IyTXlDYUwrZ0IwV1lo?=
 =?utf-8?B?eTArS1lVL0FQMlIxclpHTDhEelZlY1FUTUpYUDdZRFVLZDhselZZcWNGcGx0?=
 =?utf-8?B?T0VpT0pLbmFDWHJFQmF6NlgxUVpqRWM5QVJERGxQMk5GVUZJM0xTd2F6UnJM?=
 =?utf-8?B?YW9LNXdOUXhFanV0OVpOdzIzQzh1ZzhkUDV4QmNzUlU5LzBja3J3MHJieE9J?=
 =?utf-8?B?N3U1N3JWdHUxaE5XZitsQjV2cVpKTjRNOXc0b0xTY3VlNTdYTG9xYUNjcU5r?=
 =?utf-8?B?U3JVdlpvUk1VQVpFcU9nNUNUeGxKT0k4ZURYay9KVk5zazhzUG5qU01wZk91?=
 =?utf-8?B?M0dEdXhla3VJb25EZmVXYlZLemYrYVVwRzl2dTNTRCtZbEE0VDZ3SXNBajhi?=
 =?utf-8?B?WVhVK2NLSWd2VDRQMHJkU0FzR1NxUE1xaGliVFJTaGgreWp2MjdTYmkyZ1Bp?=
 =?utf-8?B?TCtqZHV2YVJad0NVSzlCaldIUmQ0WWE0a2lkUzA1cEJiRXZwVm5LRXB3UDd2?=
 =?utf-8?B?Vmd0akw2a2FQY1ppRDdmazlxb29oakZPVFNQTFM1d1UyZDlEZlJNSXViTk9D?=
 =?utf-8?B?cXVjNFA3NVJGTitiU2pYQ05Tb2Z0aXhVeFEzdUd2bmNCYWlmdlRlL3YydXE3?=
 =?utf-8?B?NWJrUDJFMEMxb09SMU9GRitMWGdvYlBxL3lmNkRhTGpvSlhnSnZFVWtUdUVm?=
 =?utf-8?Q?m0SLLtjaNZnnUFg3o5s+qHs=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9b0c4ecf-74d7-4814-fc86-08d9ffe2e9ce
X-MS-Exchange-CrossTenant-AuthSource: AM7PR04MB7046.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2022 02:33:50.1702
 (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: FfBlBwKO6usX1F3ku0bVOA1TtgcPQbyYt2am15wODYIhVsW4Js17JmkOxSvereipPxkkOF8DHro3Utvxk+3lzQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0402MB2785
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,T_SCC_BODY_TEXT_LINE
        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 Mon, 2022-02-07 at 13:46 +0800, Liu Ying wrote:
> If the CRTC state is already inactive, it doesn't make sense to trigger
> the entry timer for self refresh work to make the display enter self
> refresh mode, because the disabled CRTC hints that either the entire
> display pipeline is disabled or the previous atomic commit is triggered
> by the self refresh work(the CRTC is disabled, while the relevant encoder
> and bridges could be disabled or not depending on the drivers).
> 
> Cc: Rob Clark <robdclark@chromium.org>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Zain Wang <wzz@rock-chips.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Signed-off-by: Liu Ying <victor.liu@nxp.com>

Gentle ping...

Thanks,
Liu Ying

> ---
>  drivers/gpu/drm/drm_self_refresh_helper.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_self_refresh_helper.c b/drivers/gpu/drm/drm_self_refresh_helper.c
> index dd33fec5aabd..e76eb3cd22c7 100644
> --- a/drivers/gpu/drm/drm_self_refresh_helper.c
> +++ b/drivers/gpu/drm/drm_self_refresh_helper.c
> @@ -204,8 +204,12 @@ void drm_self_refresh_helper_alter_state(struct drm_atomic_state *state)
>  		struct drm_self_refresh_data *sr_data;
>  		unsigned int delay;
>  
> -		/* Don't trigger the entry timer when we're already in SR */
> -		if (crtc_state->self_refresh_active)
> +		/*
> +		 * Don't trigger the entry timer when we're already inactive.
> +		 * Note that the inactive state hints that either we're already
> +		 * in SR or the entire display pipeline is already disabled.
> +		 */
> +		if (!crtc_state->active)
>  			continue;
>  
>  		sr_data = crtc->self_refresh_data;