From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 05/13] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon Date: Wed, 20 Mar 2013 14:31:38 +0200 Message-ID: <1363782698.13528.22.camel@intelbox> References: <1361309508-4901-1-git-send-email-jbarnes@virtuousgeek.org> <1361309508-4901-6-git-send-email-jbarnes@virtuousgeek.org> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 6E5C3E6608 for ; Wed, 20 Mar 2013 05:31:41 -0700 (PDT) In-Reply-To: <1361309508-4901-6-git-send-email-jbarnes@virtuousgeek.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, 2013-02-19 at 13:31 -0800, Jesse Barnes wrote: > From: Chris Wilson > > Signed-off-by: Chris Wilson > Signed-off-by: Jesse Barnes > --- > 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)