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

Imre Deak imre.deak at intel.com
Wed Mar 20 13:31:38 CET 2013


On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote:
> From: Chris Wilson <chris at chris-wilson.co.uk>
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_dma.c      |    8 +-
>  drivers/gpu/drm/i915/i915_drv.h      |    2 +-
>  drivers/gpu/drm/i915/intel_display.c |   14 +-
>  drivers/gpu/drm/i915/intel_drv.h     |    4 +
>  drivers/gpu/drm/i915/intel_fb.c      |  305 +++++++++++++++++++++++++++++++---
>  5 files changed, 306 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 4fa6beb..f2b7db7 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1273,6 +1273,7 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
>  static int i915_load_modeset_init(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	bool was_vga_enabled;
>  	int ret;
>  
>  	ret = intel_parse_bios(dev);
> @@ -1309,7 +1310,11 @@ static int i915_load_modeset_init(struct drm_device *dev)
>  
>  	/* Important: The output setup functions called by modeset_init need
>  	 * working irqs for e.g. gmbus and dp aux transfers. */
> -	intel_modeset_init(dev);
> +	intel_modeset_init(dev, &was_vga_enabled);
> +
> +	/* Wrap existing BIOS mode configuration prior to GEM takeover */
> +	if (!was_vga_enabled)
> +		intel_fbdev_init_bios(dev);
>  
>  	ret = i915_gem_init(dev);
>  	if (ret)
> @@ -1323,6 +1328,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
>  	/* FIXME: do pre/post-mode set stuff in core KMS code */
>  	dev->vblank_disable_allowed = 1;
>  
> +	/* Install a default KMS/GEM fbcon if we failed to wrap the BIOS fb */
>  	ret = intel_fbdev_init(dev);
>  	if (ret)
>  		goto cleanup_gem;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9b5478f..30cf7e6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1811,7 +1811,7 @@ static inline void intel_unregister_dsm_handler(void) { return; }
>  
>  /* modesetting */
>  extern void intel_modeset_init_hw(struct drm_device *dev);
> -extern void intel_modeset_init(struct drm_device *dev);
> +extern void intel_modeset_init(struct drm_device *dev, bool *was_vga_enabled);
>  extern void intel_modeset_gem_init(struct drm_device *dev);
>  extern void intel_modeset_cleanup(struct drm_device *dev);
>  extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dc58b01..9793e66 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8735,12 +8735,17 @@ static void intel_init_quirks(struct drm_device *dev)
>  }
>  
>  /* Disable the VGA plane that we never use */
> -static void i915_disable_vga(struct drm_device *dev)
> +static bool i915_disable_vga(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	bool was_enabled;
>  	u8 sr1;
>  	u32 vga_reg = i915_vgacntrl_reg(dev);
>  
> +	was_enabled = !(I915_READ(vga_reg) & VGA_DISP_DISABLE);
> +	DRM_DEBUG_KMS("VGA output is currently %s\n",
> +		      was_enabled ? "enabled" : "disabled");
> +
>  	vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO);
>  	outb(SR01, VGA_SR_INDEX);
>  	sr1 = inb(VGA_SR_DATA);
> @@ -8750,6 +8755,8 @@ static void i915_disable_vga(struct drm_device *dev)
>  
>  	I915_WRITE(vga_reg, VGA_DISP_DISABLE);
>  	POSTING_READ(vga_reg);
> +
> +	return was_enabled;
>  }
>  
>  void intel_modeset_init_hw(struct drm_device *dev)
> @@ -8765,7 +8772,8 @@ void intel_modeset_init_hw(struct drm_device *dev)
>  	mutex_unlock(&dev->struct_mutex);
>  }
>  
> -void intel_modeset_init(struct drm_device *dev)
> +void intel_modeset_init(struct drm_device *dev,
> +			bool *was_vga_enabled)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int i, ret;
> @@ -8812,7 +8820,7 @@ void intel_modeset_init(struct drm_device *dev)
>  	intel_pch_pll_init(dev);
>  
>  	/* Just disable it once at startup */
> -	i915_disable_vga(dev);
> +	*was_vga_enabled = i915_disable_vga(dev);
>  	intel_setup_outputs(dev);
>  
>  	/* Just in case the BIOS is doing something questionable. */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f93653d..9cf794f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -146,6 +146,8 @@ struct intel_fbdev {
>  	struct intel_framebuffer ifb;
>  	struct list_head fbdev_list;
>  	struct drm_display_mode *our_mode;
> +	bool stolen;
> +	int preferred_bpp;
>  };
>  
>  struct intel_encoder {
> @@ -212,6 +214,7 @@ struct intel_crtc {
>  	enum plane plane;
>  	enum transcoder cpu_transcoder;
>  	u8 lut_r[256], lut_g[256], lut_b[256];
> +	bool mode_valid;
>  	/*
>  	 * Whether the crtc and the connected output pipeline is active. Implies
>  	 * that crtc->enabled is set, i.e. the current mode configuration has
> @@ -616,6 +619,7 @@ extern 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);
> +extern void intel_fbdev_init_bios(struct drm_device *dev);
>  extern int intel_fbdev_init(struct drm_device *dev);
>  extern void intel_fbdev_initial_config(struct drm_device *dev);
>  extern void intel_fbdev_fini(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
> index 5afc31b..4ca2ee4 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -131,7 +131,6 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
>  	struct drm_device *dev = ifbdev->helper.dev;
>  	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
>  	struct drm_i915_gem_object *obj;
> -	struct fb_info *info;
>  	int size, ret;
>  
>  	/* we don't do packed 24bpp */
> @@ -176,14 +175,7 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
>  		      mode_cmd.width, mode_cmd.height,
>  		      obj->gtt_offset, obj);
>  
> -	info = intelfb_create_info(ifbdev);
> -	if (info == NULL) {
> -		ret = -ENOMEM;
> -		goto out_unpin;
> -	}
> -
>  	mutex_unlock(&dev->struct_mutex);
> -	vga_switcheroo_client_fb_set(dev->pdev, info);
>  	return 0;
>  
>  out_unpin:
> @@ -200,17 +192,92 @@ static int intel_fb_find_or_create_single(struct drm_fb_helper *helper,
>  {
>  	struct intel_fbdev *ifbdev = (struct intel_fbdev *)helper;
>  	int new_fb = 0;
> -	int ret;
>  
>  	if (!helper->fb) {
> -		ret = intelfb_create(ifbdev, sizes);
> -		if (ret)
> -			return ret;
> +		struct fb_info *info;
> +
> +		if (!ifbdev->stolen) {
> +			int ret = intelfb_create(ifbdev, sizes);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		info = intelfb_create_info(ifbdev);
> +		if (info == NULL) {
> +			DRM_DEBUG_KMS("fb creation failed\n");
> +			return -ENOMEM;
> +		}
> +		vga_switcheroo_client_fb_set(helper->dev->pdev, info);
> +
>  		new_fb = 1;
>  	}
> +
>  	return new_fb;
>  }
>  
> +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)->mode_valid) {
> +			DRM_DEBUG_KMS("connector %s on crtc %d has an invalid mode, aborting\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,
> @@ -242,23 +309,215 @@ static void intel_fbdev_destroy(struct drm_device *dev,
>  	}
>  }
>  
> -int intel_fbdev_init(struct drm_device *dev)
> +static bool pipe_enabled(struct drm_i915_private *dev_priv, enum pipe pipe)
>  {
> +	enum transcoder cpu_transcoder =
> +		intel_pipe_to_cpu_transcoder(dev_priv, pipe);
> +	return !!(I915_READ(PIPECONF(cpu_transcoder)) & PIPECONF_ENABLE);
> +}
> +
> +/*
> + * 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;
> -	drm_i915_private_t *dev_priv = dev->dev_private;
> -	int ret;
> +	struct drm_crtc *crtc;
> +	struct drm_mode_fb_cmd2 mode_cmd = { 0 };
> +	struct drm_i915_gem_object *obj;
> +	u32 obj_offset = 0;
> +	int mode_bpp = 0;
> +	u32 active = 0;
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +		int pipe = intel_crtc->pipe, plane = intel_crtc->plane;
> +		u32 val, bpp, offset, format;
> +		int pitch, width, height;
> +
> +		if (!pipe_enabled(dev_priv, pipe)) {
> +			DRM_DEBUG_KMS("pipe %c not active, skipping\n",
> +				      pipe_name(pipe));
> +			continue;
> +		}
> +
> +		val = I915_READ(DSPCNTR(plane));
> +
> +		if (INTEL_INFO(dev)->gen >= 4) {
> +			if (val & DISPPLANE_TILED) {
> +				DRM_DEBUG_KMS("tiled BIOS fb?\n");
> +				continue; /* unexpected! */
> +			}
> +		}
> +
> +		switch (val & DISPPLANE_PIXFORMAT_MASK) {
> +		case DISPPLANE_YUV422:
> +		default:
> +			DRM_DEBUG_KMS("pipe %c unsupported pixel format %x, skipping\n",
> +				      pipe_name(pipe), (val & DISPPLANE_PIXFORMAT_MASK) >> 26);
> +			continue;
> +		case DISPPLANE_8BPP:
> +			format = DRM_FORMAT_C8;
> +			bpp = 8;
> +			break;
> +		case DISPPLANE_BGRX555:
> +			format = DRM_FORMAT_XRGB1555;
> +			bpp = 16;
> +			break;
> +		case DISPPLANE_BGRX565:
> +			format = DRM_FORMAT_RGB565;
> +			bpp = 16;
> +			break;
> +		case DISPPLANE_BGRX888:
> +			format = DRM_FORMAT_XRGB8888;
> +			bpp = 32;
> +			break;
> +		}
> +
> +		if (mode_cmd.pixel_format == 0) {
> +			mode_bpp = bpp;
> +			mode_cmd.pixel_format = format;
> +		}
> +
> +		if (mode_cmd.pixel_format != format) {
> +			DRM_DEBUG_KMS("pipe %c has format/bpp (%d, %d) mismatch: skipping\n",
> +				      pipe_name(pipe), format, bpp);
> +			continue;
> +		}
> +
> +		if (INTEL_INFO(dev)->gen >= 4) {
> +			if (I915_READ(DSPTILEOFF(plane))) {
> +				DRM_DEBUG_KMS("pipe %c is offset: skipping\n",
> +					      pipe_name(pipe));
> +				continue;
> +			}
> +
> +			offset = I915_READ(DSPSURF(plane));
> +		} else
> +			offset = I915_READ(DSPADDR(plane));

Nitpick: the second branch should be inside { } too.

> +		if (!obj_offset)
> +			obj_offset = offset;
> +
> +		pitch = I915_READ(DSPSTRIDE(plane));
> +		if (mode_cmd.pitches[0] == 0)
> +			mode_cmd.pitches[0] = pitch;
> +
> +		if (offset != obj_offset || pitch != mode_cmd.pitches[0]) {
> +			DRM_DEBUG_KMS("multiple pipe setup not in clone mode, sjipping\n");

s/sjipping/skipping/

> +			continue;
> +		}
> +
> +		val = I915_READ(PIPESRC(pipe));
> +		width = ((val >> 16) & 0xfff) + 1;
> +		height = ((val >> 0) & 0xfff) + 1;
> +
> +		DRM_DEBUG_KMS("Found active pipe [%d/%d]: size=%dx%d@%d, offset=%x\n",
> +			      pipe, plane, width, height, bpp, offset);
> +
> +		if (width > mode_cmd.width)
> +			mode_cmd.width = width;
> +
> +		if (height > mode_cmd.height)
> +			mode_cmd.height = height;
> +
> +		active |= 1 << pipe;
> +	}
> +
> +	if (active == 0) {
> +		DRM_DEBUG_KMS("no active pipes found, not using BIOS config\n");
> +		return;
> +	}
>  
>  	ifbdev = kzalloc(sizeof(struct intel_fbdev), GFP_KERNEL);
> -	if (!ifbdev)
> -		return -ENOMEM;
> +	if (ifbdev == NULL) {
> +		DRM_DEBUG_KMS("failed to alloc intel fbdev\n");
> +		return;
> +	}
>  
> -	dev_priv->fbdev = ifbdev;
> +	ifbdev->stolen = true;
> +	ifbdev->preferred_bpp = mode_bpp;
>  	ifbdev->helper.funcs = &intel_fb_helper_funcs;
> +	ifbdev->helper.funcs->initial_config = intel_fb_initial_config;
> +
> +	/* assume a 1:1 linear mapping between stolen and GTT */
> +	obj = i915_gem_object_create_stolen_for_preallocated(dev,
> +							     obj_offset,
> +							     obj_offset,
> +							     ALIGN(mode_cmd.pitches[0] * mode_cmd.height, PAGE_SIZE));
> +	if (obj == NULL) {
> +		DRM_DEBUG_KMS("failed to create stolen fb\n");
> +		goto out_free_ifbdev;
> +	}
> +
> +	if (intel_framebuffer_init(dev, &ifbdev->ifb, &mode_cmd, obj)) {
> +		DRM_DEBUG_KMS("intel fb init failed\n");
> +		goto out_unref_obj;
> +	}
> +
> +	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> +		int ret;
> +
> +		if ((active & (1 << to_intel_crtc(crtc)->pipe)) == 0)
> +			continue;
> +
> +		ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
> +		if (ret)
> +			goto out_unref_obj;

Since fb will be destroyed, is it ok to leave references to it in
crtc->fb set in previous iterations?

> +
> +		crtc->fb = &ifbdev->ifb.base;
> +	}
> +
> +	dev_priv->fbdev = ifbdev;
> +
> +	DRM_DEBUG_KMS("using BIOS fb for initial console\n");
> +	return;
> +
> +out_unref_obj:
> +	drm_gem_object_unreference(&obj->base);
> +out_free_ifbdev:
> +	kfree(ifbdev);
> +}
> +
> +int intel_fbdev_init(struct drm_device *dev)
> +{
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	struct intel_fbdev *ifbdev;
> +	int ret;
> +
> +	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;
> +	}
>  
>  	ret = drm_fb_helper_init(dev, &ifbdev->helper,
> -				 dev_priv->num_pipe,
> -				 INTELFB_CONN_LIMIT);
> +			dev_priv->num_pipe,
> +			INTELFB_CONN_LIMIT);

Unnecessary w/s change.

>  	if (ret) {
> +		dev_priv->fbdev = NULL;
>  		kfree(ifbdev);
>  		return ret;
>  	}
> @@ -271,9 +530,10 @@ int intel_fbdev_init(struct drm_device *dev)
>  void intel_fbdev_initial_config(struct drm_device *dev)
>  {
>  	drm_i915_private_t *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)
> @@ -301,7 +561,8 @@ MODULE_LICENSE("GPL and additional rights");
>  void intel_fb_output_poll_changed(struct drm_device *dev)
>  {
>  	drm_i915_private_t *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_fb_restore_mode(struct drm_device *dev)





More information about the Intel-gfx mailing list