[Intel-gfx] [PATCH 4/5] drm/i915: Wrap the preallocated BIOS framebuffer and preserve for KMS fbcon
Jesse Barnes
jbarnes at virtuousgeek.org
Wed Nov 13 23:08:19 CET 2013
On Wed, 13 Nov 2013 14:05:44 -0800
Bob Paauwe <bob.j.paauwe at intel.com> wrote:
> 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.
Yeah it could be clearer. But the fbdev_init_bios function just
modifies the actual intel_fb_helper_funcs struct, so we can still
assign the address and not clobber what it did.
But it is confusing, I'll come up with something better. The comment
could use some updating too, as this is a twisty maze between
drm_fb_helper and our code.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
More information about the Intel-gfx
mailing list