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 peter.ujfalusi@ti.com

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