From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757432AbcH2Nax (ORCPT ); Mon, 29 Aug 2016 09:30:53 -0400 Received: from arroyo.ext.ti.com ([198.47.19.12]:48587 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756604AbcH2Naw (ORCPT ); Mon, 29 Aug 2016 09:30:52 -0400 Subject: Re: [PATCH] drm: drm_probe_helper: Fix output_poll_work scheduling To: , , , , References: <20160829095022.12160-1-peter.ujfalusi@ti.com> <20160829125857.GK10980@phenom.ffwll.local> From: Peter Ujfalusi Message-ID: Date: Mon, 29 Aug 2016 16:30:44 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160829125857.GK10980@phenom.ffwll.local> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/29/16 15:58, Daniel Vetter wrote: > On Mon, Aug 29, 2016 at 12:50:22PM +0300, Peter Ujfalusi wrote: >> drm_kms_helper_poll_enable_locked() should check if we have delayed event >> pending and if we have, schedule the work to run without delay. >> >> Currently the output_poll_work is only scheduled if any of the connectors >> have DRM_CONNECTOR_POLL_CONNECT or DRM_CONNECTOR_POLL_DISCONNECT with >> DRM_OUTPUT_POLL_PERIOD delay. It does not matter if we have delayed event >> already registered to be handled. The detection will be delayd by >> DRM_OUTPUT_POLL_PERIOD in any case. >> Furthermore if none of the connectors are marked as POLL_CONNECT or >> POLL_DISCONNECT because all connectors are either POLL_HPD or they are >> always connected: the output_poll_work will not run at all even if we >> have delayed event marked. >> >> When none of the connectors require polling, their initial status change >> from unknown to connected/disconnected is not going to be handled until >> the first kms application starts or if we have fb console enabled. >> >> With this change we can react more quickly to output status changes after >> enabling the poll but most importantly it will correct the behavior when >> none of the connector have POLL_CONNECT or POLL_DISCONNECT flag set - they >> are either POLL_HPD or the .polled is 0. The initial status change is going >> to be handled correctly as well. > > Don't we need to set delayed_event somewhere too first? At least I don't > really understand how this speeds things up The delayed_event is set first in the drm_helper_probe_single_connector_modes_merge_bits() when the force is not set. The drm_kms_helper_poll_init() is called by drivers when they have their connectors set up. So when probing the drivers we will first set up the connectors, they will move from connector_status_unknown to connected or disconnected and the delayed_event is set. Later the drm_kms_helper_poll_init() is called and that will schedule the work with DRM_OUTPUT_POLL_PERIOD. So even if we now know the state of the connectors, they will be checked by the poll_work when the work is running with a delay. If we set the delay to 0 the detection will run much faster. Is this makes more sense? > ... The patch itself looks > like a valid bugfix, but the description confuses me a bit. I think if you > delete the above paragraph (starting with "With this change we can react > ...") then it's all good. I can drop the last paragraph. > -Daniel > >> >> Signed-off-by: Peter Ujfalusi >> --- >> drivers/gpu/drm/drm_probe_helper.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c >> index a0df377d7d1c..f6b64d7d3528 100644 >> --- a/drivers/gpu/drm/drm_probe_helper.c >> +++ b/drivers/gpu/drm/drm_probe_helper.c >> @@ -129,6 +129,7 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev) >> { >> bool poll = false; >> struct drm_connector *connector; >> + unsigned long delay = DRM_OUTPUT_POLL_PERIOD; >> >> WARN_ON(!mutex_is_locked(&dev->mode_config.mutex)); >> >> @@ -141,8 +142,13 @@ void drm_kms_helper_poll_enable_locked(struct drm_device *dev) >> poll = true; >> } >> >> + if (dev->mode_config.delayed_event) { >> + poll = true; >> + delay = 0; >> + } >> + >> if (poll) >> - schedule_delayed_work(&dev->mode_config.output_poll_work, DRM_OUTPUT_POLL_PERIOD); >> + schedule_delayed_work(&dev->mode_config.output_poll_work, delay); >> } >> EXPORT_SYMBOL(drm_kms_helper_poll_enable_locked); >> >> -- >> 2.9.3 >> > -- Péter