Re: [PATCH v2] drm: drm_probe_helper: Fix output_poll_work scheduling

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 31 Aug 2016, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Aug 31, 2016 at 02:09:05PM +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.
>> 
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>> Hi,
>> 
>> Changes since v1:
>> - dropped the last paragraph from the commit message.
>
> I added a few more words to the commit message to explain when exactly
> this is a problem and applied your patch to drm-misc.

Hi Peter, sadly looks like this regresses users out there [1]. Seems to
be a reliable bisect. We need to have this fixed or reverted.

BR,
Jani.


[1] https://bugs.freedesktop.org/show_bug.cgi?id=98690



>
> Thanks, Daniel
>> 
>> Regards,
>> Peter
>> 
>>  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
>> 

-- 
Jani Nikula, Intel Open Source Technology Center