Re: [RFC][PATCH 07/11] drm/probe-helper: Avoid iterating the list twice on ww backoff

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Feb 27, 2018 at 02:56:56PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> If we have to backoff there's no point in going over the mode list again
> to mark all the modes as stale. We can defer that until we're ready to
> refresh the mode list. Avoids multiple list walks if we have to do the
> locking backoff.
> 
> Cc: Keith Packard <keithp@keithp.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Some drivers add modes from their ->detect callbacks I think, won't this
break them? At least I'm never sure about what different drivers are
supposed to do in each case.

Otoh the kerneldoc is fairly clear, and this patch is easy to revert, and
it might help in forcing drivers to be more consistent.

If you add a note about my concerns (as a hint in case we bisect a
regression to this patch), then this is:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>


> ---
>  drivers/gpu/drm/drm_probe_helper.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 527743394150..7dc7e635d7e4 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -415,10 +415,6 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>  	} else
>  		WARN_ON(ret < 0);
>  
> -	/* set all old modes to the stale state */
> -	list_for_each_entry(mode, &connector->modes, head)
> -		mode->status = MODE_STALE;
> -
>  	old_status = connector->status;
>  
>  	if (connector->force) {
> @@ -472,6 +468,10 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>  
>  	dev->mode_config.poll_running = drm_kms_helper_poll;
>  
> +	/* set all old modes to the stale state */
> +	list_for_each_entry(mode, &connector->modes, head)
> +		mode->status = MODE_STALE;
> +
>  	if (connector->status == connector_status_disconnected) {
>  		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n",
>  			connector->base.id, connector->name);
> -- 
> 2.13.6
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx