[Intel-gfx] [PATCH 4/5] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon

Bob Paauwe bob.j.paauwe at intel.com
Wed Nov 13 23:05:44 CET 2013


On Wed, 13 Nov 2013 10:20:47 -0800
Jesse Barnes <jbarnes at virtuousgeek.org> wrote:

> Retrieve current framebuffer config info from the regs and create an fb
> object for the buffer the BIOS or boot loader left us.  This should
> allow for smooth transitions to userspace apps once we finish the
> initial configuration construction.
> 
> v2: check for non-native modes and adjust (Jesse)
>     fixup aperture and cmap frees (Imre)
>     use unlocked unref if init_bios fails (Jesse)
>     fix curly brace around DSPADDR check (Imre)
>     comment failure path for pin_and_fence (Imre)
> v3: fixup fixup of aperture frees (Chris)
> v4: update to current bits (locking & pin_and_fence hack) (Jesse)
> v5: move fb config fetch to display code (Jesse)
>     re-order hw state readout on initial load to suit fb inherit (Jesse)
>     re-add pin_and_fence in fbdev code to make sure we refcount properly (Je
> v6: rename to plane_config (Daniel)
>     check for valid object when initializing BIOS fb (Jesse)
>     split from plane_config readout and other display changes (Jesse)
> 
> Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.c    |   5 +
>  drivers/gpu/drm/i915/i915_drv.h    |   1 +
>  drivers/gpu/drm/i915/intel_drv.h   |   2 +
>  drivers/gpu/drm/i915/intel_fbdev.c | 187 +++++++++++++++++++++++++++++++++++--
>  4 files changed, 189 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 30f4fe7..bcad343 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -154,6 +154,11 @@ module_param_named(prefault_disable, i915_prefault_disable, bool, 0600);
>  MODULE_PARM_DESC(prefault_disable,
>  		"Disable page prefaulting for pread/pwrite/reloc (default:false). For developers only.");
>  
> +bool i915_use_bios_fb __read_mostly = 1;
> +module_param_named(use_bios_fb, i915_use_bios_fb, bool, 0600);
> +MODULE_PARM_DESC(use_bios_fb,
> +		 "Use BIOS allocated framebuffer for fbcon (default: true)");
> +
>  static struct drm_driver driver;
>  #if IS_ENABLED(CONFIG_AGP_INTEL)
>  extern int intel_agp_enabled;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index db6821a..500ccc7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1877,6 +1877,7 @@ extern bool i915_fastboot __read_mostly;
>  extern int i915_enable_pc8 __read_mostly;
>  extern int i915_pc8_timeout __read_mostly;
>  extern bool i915_prefault_disable __read_mostly;
> +extern bool i915_use_bios_fb __read_mostly;
>  
>  extern int i915_suspend(struct drm_device *dev, pm_message_t state);
>  extern int i915_resume(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cb72304..61dfffd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -113,6 +113,7 @@ struct intel_fbdev {
>  	struct intel_framebuffer ifb;
>  	struct list_head fbdev_list;
>  	struct drm_display_mode *our_mode;
> +	int preferred_bpp;
>  };
>  
>  struct intel_encoder {
> @@ -666,6 +667,7 @@ int intel_framebuffer_init(struct drm_device *dev,
>  			   struct intel_framebuffer *ifb,
>  			   struct drm_mode_fb_cmd2 *mode_cmd,
>  			   struct drm_i915_gem_object *obj);
> +void intel_fbdev_init_bios(struct drm_device *dev);
>  void intel_framebuffer_fini(struct intel_framebuffer *fb);
>  void intel_prepare_page_flip(struct drm_device *dev, int plane);
>  void intel_finish_page_flip(struct drm_device *dev, int pipe);
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 8fa946b..adf92dd 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -238,6 +238,69 @@ static void intel_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
>  	*blue = intel_crtc->lut_b[regno] << 8;
>  }
>  
> +static struct drm_fb_helper_crtc *
> +intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
> +{
> +	int i;
> +
> +	for (i = 0; i < fb_helper->crtc_count; i++)
> +		if (fb_helper->crtc_info[i].mode_set.crtc == crtc)
> +			return &fb_helper->crtc_info[i];
> +
> +	return NULL;
> +}
> +
> +static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> +				    struct drm_fb_helper_crtc **crtcs,
> +				    struct drm_display_mode **modes,
> +				    bool *enabled, int width, int height)
> +{
> +	int i;
> +
> +	for (i = 0; i < fb_helper->connector_count; i++) {
> +		struct drm_connector *connector;
> +		struct drm_encoder *encoder;
> +
> +		connector = fb_helper->connector_info[i]->connector;
> +		if (!enabled[i]) {
> +			DRM_DEBUG_KMS("connector %d not enabled, skipping\n",
> +				      connector->base.id);
> +			continue;
> +		}
> +
> +		encoder = connector->encoder;
> +		if (!encoder || !encoder->crtc) {
> +			DRM_DEBUG_KMS("connector %d has no encoder or crtc, skipping\n",
> +				      connector->base.id);
> +			continue;
> +		}
> +
> +		if (WARN_ON(!encoder->crtc->enabled)) {
> +			DRM_DEBUG_KMS("connector %s on crtc %d has inconsistent state, aborting\n",
> +				      drm_get_connector_name(connector),
> +				      encoder->crtc->base.id);
> +			return false;
> +		}
> +
> +		if (!to_intel_crtc(encoder->crtc)->active) {
> +			DRM_DEBUG_KMS("connector %s on inactive crtc %d, borting\n",
> +				      drm_get_connector_name(connector),
> +				      encoder->crtc->base.id);
> +			return false;
> +		}
> +
> +		modes[i] = &encoder->crtc->mode;
> +		crtcs[i] = intel_fb_helper_crtc(fb_helper, encoder->crtc);
> +
> +		DRM_DEBUG_KMS("connector %s on crtc %d: %s\n",
> +			      drm_get_connector_name(connector),
> +			      encoder->crtc->base.id,
> +			      modes[i]->name);
> +	}
> +
> +	return true;
> +}
> +
>  static struct drm_fb_helper_funcs intel_fb_helper_funcs = {
>  	.gamma_set = intel_crtc_fb_gamma_set,
>  	.gamma_get = intel_crtc_fb_gamma_get,
> @@ -264,23 +327,133 @@ static void intel_fbdev_destroy(struct drm_device *dev,
>  	intel_framebuffer_fini(&ifbdev->ifb);
>  }
>  
> +/*
> + * Try to read the BIOS display configuration and use it for the initial
> + * fb configuration.
> + *
> + * The BIOS or boot loader will generally create an initial display
> + * configuration for us that includes some set of active pipes and displays.
> + * This routine tries to figure out which pipes are active, what resolutions
> + * are being displayed, and then allocates a framebuffer and initial fb
> + * config based on that data.
> + *
> + * If the BIOS or boot loader leaves the display in VGA mode, there's not
> + * much we can do; switching out of that mode involves allocating a new,
> + * high res buffer, and also recalculating bandwidth requirements for the
> + * new bpp configuration.
> + *
> + * However, if we're loaded into an existing, high res mode, we should
> + * be able to allocate a buffer big enough to handle the largest active
> + * mode, create a mode_set for it, and pass it to the fb helper to create
> + * the configuration.
> + */
> +void intel_fbdev_init_bios(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_fbdev *ifbdev;
> +	struct drm_crtc *crtc;
> +	struct intel_crtc *intel_crtc;
> +	struct intel_plane_config *plane_config;
> +	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> +	u32 active = 0;
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		intel_crtc = to_intel_crtc(crtc);
> +
> +		if (!intel_crtc->active) {
> +			DRM_DEBUG_KMS("pipe %c not active, skipping\n",
> +				      pipe_name(intel_crtc->pipe));
> +			continue;
> +		}
> +
> +		active |= 1 << intel_crtc->pipe;
> +		break;
> +	}
> +
> +	plane_config = &intel_crtc->plane_config;
> +
> +	if (active == 0 || !plane_config->obj) {
> +		DRM_DEBUG_KMS("no active pipes found, not using BIOS config\n");
> +		return;
> +	}
> +
> +	ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);
> +	if (ifbdev == NULL) {
> +		DRM_DEBUG_KMS("failed to alloc intel fbdev\n");
> +		return;
> +	}
> +
> +	ifbdev->preferred_bpp = plane_config->bpp;
> +	ifbdev->helper.funcs = &intel_fb_helper_funcs;
> +	ifbdev->helper.funcs->initial_config = intel_fb_initial_config;
> +
> +	mode_cmd.pixel_format = intel_format_to_fourcc(plane_config->pixel_format);
> +	mode_cmd.width = plane_config->fb_width;
> +	mode_cmd.height = plane_config->fb_height;
> +	mode_cmd.pitches[0] = plane_config->pitch;
> +
> +	mutex_lock(&dev->struct_mutex);
> +
> +	if (intel_framebuffer_init(dev, &ifbdev->ifb, &mode_cmd,
> +				   plane_config->obj)) {
> +		DRM_DEBUG_KMS("intel fb init failed\n");
> +		goto out_unref_obj;
> +	}
> +
> +	/* Assuming a single fb across all pipes here */
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		if ((active & (1 << to_intel_crtc(crtc)->pipe)) == 0)
> +			continue;
> +
> +		/*
> +		 * This should only fail on the first one so we don't need
> +		 * to cleanup any secondary crtc->fbs
> +		 */
> +		if (intel_pin_and_fence_fb_obj(dev, plane_config->obj, NULL))
> +			goto out_unref_obj;
> +
> +		crtc->fb = &ifbdev->ifb.base;
> +	}
> +
> +	dev_priv->fbdev = ifbdev;
> +
> +	DRM_DEBUG_KMS("using BIOS fb for initial console\n");
> +	mutex_unlock(&dev->struct_mutex);
> +	return;
> +
> +out_unref_obj:
> +	mutex_unlock(&dev->struct_mutex);
> +	drm_gem_object_unreference_unlocked(&plane_config->obj->base);
> +	kfree(ifbdev);
> +}
> +
>  int intel_fbdev_init(struct drm_device *dev)
>  {
>  	struct intel_fbdev *ifbdev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ret;
>  
> -	ifbdev = kzalloc(sizeof(*ifbdev), GFP_KERNEL);
> -	if (!ifbdev)
> -		return -ENOMEM;
> +	if (i915_use_bios_fb)
> +		intel_fbdev_init_bios(dev);
> +
> +	if ((ifbdev = dev_priv->fbdev) == NULL) {
> +		ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);
> +		if (ifbdev == NULL)
> +			return -ENOMEM;
> +
> +		ifbdev->helper.funcs = &intel_fb_helper_funcs;
> +		ifbdev->preferred_bpp = 32;
> +
> +		dev_priv->fbdev = ifbdev;
> +	}
>  
> -	dev_priv->fbdev = ifbdev;
>  	ifbdev->helper.funcs = &intel_fb_helper_funcs;

Should helper.funcs still be set here?  It looks like it will be set by
intel_fbdev_init_bios if that is called/succeeds and otherwise set
a few lines above inside the if.  If it is set in intel_fbdev_init_bios
then this will override the change made there to the initial_config
helper.


>  
>  	ret = drm_fb_helper_init(dev, &ifbdev->helper,
>  				 INTEL_INFO(dev)->num_pipes,
>  				 4);
>  	if (ret) {
> +		dev_priv->fbdev = NULL;
>  		kfree(ifbdev);
>  		return ret;
>  	}
> @@ -293,9 +466,10 @@ int intel_fbdev_init(struct drm_device *dev)
>  void intel_fbdev_initial_config(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_fbdev *ifbdev = dev_priv->fbdev;
>  
>  	/* Due to peculiar init order wrt to hpd handling this is separate. */
> -	drm_fb_helper_initial_config(&dev_priv->fbdev->helper, 32);
> +	drm_fb_helper_initial_config(&ifbdev->helper, ifbdev->preferred_bpp);
>  }
>  
>  void intel_fbdev_fini(struct drm_device *dev)
> @@ -335,7 +509,8 @@ MODULE_LICENSE("GPL and additional rights");
>  void intel_fbdev_output_poll_changed(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
> +	if (dev_priv->fbdev)
> +		drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
>  }
>  
>  void intel_fbdev_restore_mode(struct drm_device *dev)




More information about the Intel-gfx mailing list