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@xxxxxx>
>> ---
>> 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
