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

Daniel Vetter daniel at ffwll.ch
Tue Apr 2 20:24:44 CEST 2013


On Tue, Apr 02, 2013 at 10:03:47AM -0700, Jesse Barnes wrote:
> 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)
> 
> 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      |  295 ++++++++++++++++++++++++++++++++--
>  5 files changed, 304 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index ebcfe2e..1389247 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);

I'm not really happy that the fbdev is initialized at different points
here depending upon whether. Also this seems to tie the bios fb
reconstruction quite heavily into our fbdev code ...

Can't we instead reconstruct the bios fb in intel_modeset_setup_hw_state
and assign it to each active crtc if we detect one? The fbdev init code
down below could then check whether the in-use fbs are suitable or not.

That would also make it easier to move some of the pipe state readout into
the pipe_config hw state checking/read out infrastructure. Since that's
only setup up at the end of modeset_gem_init. Or do I miss a delicate
ordering constraint here?
-Daniel

>  
>  	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 7f6452b..d32ed27 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1806,7 +1806,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 dfc8152..a55ef8f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8854,12 +8854,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);
> @@ -8869,6 +8874,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)
> @@ -8884,7 +8891,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;
> @@ -8932,7 +8940,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 f593e26..52e4924 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
> @@ -621,6 +624,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 8736a77..651077c 100644
> --- a/drivers/gpu/drm/i915/intel_fb.c
> +++ b/drivers/gpu/drm/i915/intel_fb.c
> @@ -118,8 +118,7 @@ static struct fb_info *intelfb_create_info(struct intel_fbdev *ifbdev)
>  	return info;
>  
>  err_cmap:
> -	if (info->cmap.len)
> -		fb_dealloc_cmap(&info->cmap);
> +	fb_dealloc_cmap(&info->cmap);
>  err_info:
>  	framebuffer_release(info);
>  	return NULL;
> @@ -183,9 +182,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
>  		goto out_unpin;
>  	}
>  
> -
>  	mutex_unlock(&dev->struct_mutex);
> -	vga_switcheroo_client_fb_set(dev->pdev, info);
>  	return 0;
>  
>  out_unpin:
> @@ -197,6 +194,69 @@ out:
>  	return ret;
>  }
>  
> +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,
> @@ -213,8 +273,7 @@ static void intel_fbdev_destroy(struct drm_device *dev,
>  
>  		unregister_framebuffer(info);
>  		iounmap(info->screen_base);
> -		if (info->cmap.len)
> -			fb_dealloc_cmap(&info->cmap);
> +		fb_dealloc_cmap(&info->cmap);
>  		framebuffer_release(info);
>  	}
>  
> @@ -228,23 +287,229 @@ 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));
> +		}
> +		if (!obj_offset)
> +			obj_offset = offset;
> +
> +		if (offset != obj_offset) {
> +			DRM_DEBUG_KMS("multiple pipe setup not in clone mode, skipping\n");
> +			continue;
> +		}
> +
> +		val = I915_READ(PIPESRC(pipe));
> +		width = ((val >> 16) & 0xfff) + 1;
> +		height = ((val >> 0) & 0xfff) + 1;
> +
> +		/* Adjust fitted modes */
> +		val = I915_READ(HTOTAL(pipe));
> +		if (((val & 0xffff) + 1) != width) {
> +			DRM_DEBUG_DRIVER("BIOS fb not native width (%d vs %d), overriding\n", width, (val & 0xffff) + 1);
> +			width = (val & 0xffff) + 1;
> +		}
> +		val = I915_READ(VTOTAL(pipe));
> +		if (((val & 0xffff) + 1) != height) {
> +			DRM_DEBUG_DRIVER("BIOS fb not native height (%d vs %d), overriding\n", height, (val & 0xffff) + 1);
> +			height = (val & 0xffff) + 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;
> +
> +		pitch = intel_framebuffer_pitch_for_width(width, bpp);
> +		if (pitch > mode_cmd.pitches[0])
> +			mode_cmd.pitches[0] = pitch;
> +
> +		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) {
> +		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, 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");
> +	return;
> +
> +out_unref_obj:
> +	drm_gem_object_unreference_unlocked(&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,
>  				 INTEL_INFO(dev)->num_pipes,
>  				 INTELFB_CONN_LIMIT);
>  	if (ret) {
> +		dev_priv->fbdev = NULL;
>  		kfree(ifbdev);
>  		return ret;
>  	}
> @@ -257,9 +522,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)
> @@ -287,7 +553,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)
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch



More information about the Intel-gfx mailing list