[Intel-gfx] [PATCH v2 2/3] drm: Pull together probe + setup for drm_fb_helper

[Daniel Vetter]

On Tue, Nov 29, 2016 at 12:02:16PM +0000, Chris Wilson wrote:
> drm_fb_helper_probe_connector_modes() is always called before
> drm_setup_crtcs(), so just move the call into drm_setup_crtcs for a
> small bit of code compaction.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Reviewed-by: Daniel Vetter <daniel.vetter at intel.com>

rb not entirely correct, since it's missing the crucial note I asked for
to understand things:

"Note that register_framebuffer will do a modeset (when fbcon is enabled)
and hence must be moved out of the critical section. A follow-up patch
will add new locking for the fb list, hence move all the related
registration code together."

Sean, since you reviewed all, can you pls add this note to the commit
message when merging?

Thanks, Daniel

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 37 +++++++++++--------------------------
>  1 file changed, 11 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 380a0ec01033..90da28d2fcf3 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2099,20 +2099,19 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>  	return best_score;
>  }
>  
> -static void drm_setup_crtcs(struct drm_fb_helper *fb_helper)
> +static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
> +			    u32 width, u32 height)
>  {
>  	struct drm_device *dev = fb_helper->dev;
>  	struct drm_fb_helper_crtc **crtcs;
>  	struct drm_display_mode **modes;
>  	struct drm_fb_offset *offsets;
>  	bool *enabled;
> -	int width, height;
>  	int i;
>  
>  	DRM_DEBUG_KMS("\n");
> -
> -	width = dev->mode_config.max_width;
> -	height = dev->mode_config.max_height;
> +	if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
> +		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
>  
>  	/* prevent concurrent modification of connector_count by hotplug */
>  	lockdep_assert_held(&fb_helper->dev->mode_config.mutex);
> @@ -2236,23 +2235,15 @@ int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int bpp_sel)
>  {
>  	struct drm_device *dev = fb_helper->dev;
>  	struct fb_info *info;
> -	int count = 0;
>  	int ret;
>  
>  	if (!drm_fbdev_emulation)
>  		return 0;
>  
>  	mutex_lock(&dev->mode_config.mutex);
> -	count = drm_fb_helper_probe_connector_modes(fb_helper,
> -						    dev->mode_config.max_width,
> -						    dev->mode_config.max_height);
> -	/*
> -	 * we shouldn't end up with no modes here.
> -	 */
> -	if (count == 0)
> -		dev_info(fb_helper->dev->dev, "No connectors reported connected with modes\n");
> -
> -	drm_setup_crtcs(fb_helper);
> +	drm_setup_crtcs(fb_helper,
> +			dev->mode_config.max_width,
> +			dev->mode_config.max_height);
>  	ret = drm_fb_helper_single_fb_probe(fb_helper, bpp_sel);
>  	mutex_unlock(&dev->mode_config.mutex);
>  	if (ret)
> @@ -2300,28 +2291,22 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config);
>  int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)
>  {
>  	struct drm_device *dev = fb_helper->dev;
> -	u32 max_width, max_height;
>  
>  	if (!drm_fbdev_emulation)
>  		return 0;
>  
> -	mutex_lock(&fb_helper->dev->mode_config.mutex);
> +	mutex_lock(&dev->mode_config.mutex);
>  	if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
>  		fb_helper->delayed_hotplug = true;
> -		mutex_unlock(&fb_helper->dev->mode_config.mutex);
> +		mutex_unlock(&dev->mode_config.mutex);
>  		return 0;
>  	}
>  	DRM_DEBUG_KMS("\n");
>  
> -	max_width = fb_helper->fb->width;
> -	max_height = fb_helper->fb->height;
> +	drm_setup_crtcs(fb_helper, fb_helper->fb->width, fb_helper->fb->height);
>  
> -	drm_fb_helper_probe_connector_modes(fb_helper, max_width, max_height);
> -	mutex_unlock(&fb_helper->dev->mode_config.mutex);
> +	mutex_unlock(&dev->mode_config.mutex);
>  
> -	drm_modeset_lock_all(dev);
> -	drm_setup_crtcs(fb_helper);
> -	drm_modeset_unlock_all(dev);
>  	drm_fb_helper_set_par(fb_helper->fbdev);
>  
>  	return 0;
> -- 
> 2.10.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch