[Intel-gfx] [PATCH 3/6] drm/i915: allow re-use BIOS connector config for initial fbdev config v2
Ville Syrjälä
ville.syrjala at linux.intel.com
Wed Feb 12 13:45:03 CET 2014
On Wed, Feb 12, 2014 at 01:04:28PM +0100, Daniel Vetter wrote:
> On Wed, Feb 12, 2014 at 10:19:39AM +0100, Daniel Vetter wrote:
> > On Tue, Feb 11, 2014 at 03:28:58PM -0800, Jesse Barnes wrote:
> > > 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 and connectors
> > > are active and stuffs them into the crtcs and modes array given to us by
> > > the drm_fb_helper code.
> > >
> > > The overall sequence is:
> > > intel_fbdev_init - from driver load
> > > intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data
> > > drm_fb_helper_init - build fb helper structs
> > > drm_fb_helper_single_add_all_connectors - more fb helper structs
> > > intel_fbdev_initial_config - apply the config
> > > drm_fb_helper_initial_config - call ->probe then register_framebuffer()
> > > drm_setup_crtcs - build crtc config for fbdev
> > > intel_fb_initial_config - find active connectors etc
> > > drm_fb_helper_single_fb_probe - set up fbdev
> > > intelfb_create - re-use or alloc fb, build out fbdev structs
> > >
> > > v2: use BIOS connector config unconditionally if possible (Daniel)
> > > check for crtc cloning and reject (Daniel)
> > > fix up comments (Daniel)
> > >
> > > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > > ---
> > > drivers/gpu/drm/i915/intel_display.c | 10 ++--
> > > drivers/gpu/drm/i915/intel_fbdev.c | 103 ++++++++++++++++++++++++++++++++++
> > > 2 files changed, 107 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index e800085..dea995d 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -51,7 +51,10 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
> > >
> > > static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
> > > int x, int y, struct drm_framebuffer *old_fb);
> > > -
> > > +static 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);
> > >
> > > typedef struct {
> > > int min, max;
> > > @@ -7692,11 +7695,6 @@ static struct drm_display_mode load_detect_mode = {
> > > 704, 832, 0, 480, 489, 491, 520, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC),
> > > };
> > >
> > > -static 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);
> > > -
> > > struct drm_framebuffer *
> > > __intel_framebuffer_create(struct drm_device *dev,
> > > struct drm_mode_fb_cmd2 *mode_cmd,
> > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > > index cf46273..2a0badfc 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > > @@ -242,7 +242,110 @@ 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;
> > > +}
> > > +
> > > +/*
> > > + * 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 and connectors are active
> > > + * and stuffs them into the crtcs and modes array given to us by the
> > > + * drm_fb_helper code.
> > > + *
> > > + * The overall sequence is:
> > > + * intel_fbdev_init - from driver load
> > > + * intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data
> > > + * drm_fb_helper_init - build fb helper structs
> > > + * drm_fb_helper_single_add_all_connectors - more fb helper structs
> > > + * intel_fbdev_initial_config - apply the config
> > > + * drm_fb_helper_initial_config - call ->probe then register_framebuffer()
> > > + * drm_setup_crtcs - build crtc config for fbdev
> > > + * intel_fb_initial_config - find active connectors etc
> > > + * drm_fb_helper_single_fb_probe - set up fbdev
> > > + * intelfb_create - re-use or alloc fb, build out fbdev structs
> > > + *
> > > + * Note that we don't make special consideration whether we could actually
> > > + * switch to the selected modes without a full modeset. E.g. when the display
> > > + * is in VGA mode we need to recalculate watermarks and set a new high-res
> > > + * framebuffer anyway.
> > > + */
> > > +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, j;
> > > +
> > > + for (i = 0; i < fb_helper->connector_count; i++) {
> > > + struct drm_connector *connector;
> > > + struct drm_encoder *encoder;
> > > + struct drm_fb_helper_crtc *new_crtc;
> > > +
> > > + 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) {
> >
> > You've missed the encoder->crtc change here from my review. I've converted
> > it into a WARN_ON since our state sanitizer shouldn't let anything like
> > this get through.
> >
> > Otherwise merged the first 3 patches in this series to dinq.
>
> Ville complained on irc that his box now bots into a 640x480 console. So I
> think we need to add another check to compare the selected mode with the
> preferred mode which fbcon would have picke to make sure that htotal and
> vtotal match. Timings can obviously differe a bit.
>
> Another regression is that now we don't parse/obey video= cmdline options
> any more. This is fairly important since the enable/disable flags won't be
> parsed any more so will upset Alan and his T100 again ;-) That itself is a
> bit a design goof-up since with CONFIG_FBDEV=n that'll break, too. But
> another issue to resolve that.
BTW another funky issue I noticed about the video= option is that if you
specify a mode that's not part of the set of modes added by
drm_add_modes_noedid(), then fbcon will happily use the specified mode,
but when X starts the mode gets pruned from the mode list. It would
seem better to keep the user specified mode on the list, no matter what.
--
Ville Syrjälä
Intel OTC
More information about the Intel-gfx
mailing list