On Wed, 5 Mar 2014 19:34:45 +0100 Daniel Vetter <dan...@ffwll.ch> wrote:
> On Wed, Mar 05, 2014 at 08:27:08AM -0800, Jesse Barnes wrote: > > On Tue, 4 Mar 2014 22:08:12 +0100 > > Daniel Vetter <dan...@ffwll.ch> wrote: > > > > > On Tue, Mar 04, 2014 at 12:33:01PM -0800, Jesse Barnes wrote: > > > > On Tue, 4 Mar 2014 21:08:42 +0100 > > > > Daniel Vetter <daniel.vet...@ffwll.ch> wrote: > > > > > > > > > Both Ville and QA rather immediately complained that with the new > > > > > initial_config logic from Jesse not all outputs get enabled. Since the > > > > > fbdev emulation pretty much tries to always enable as many outputs as > > > > > possible (it even has hotplug handling and all that) fall back if more > > > > > outputs could have been enabled. > > > > > > > > > > v2: Fix up my confusion about what enabled means - it's passed from > > > > > the fbdev helper, we need to check for a non-zero connector->encoder > > > > > link. Spotted by Ville. > > > > > > > > > > Cc: Jesse Barnes <jbar...@virtuousgeek.org> > > > > > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com> > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75552 > > > > > Tested-by: Ville Syrjälä <ville.syrj...@linux.intel.com> > > > > > Signed-off-by: Daniel Vetter <daniel.vet...@ffwll.ch> > > > > > --- > > > > > drivers/gpu/drm/i915/intel_fbdev.c | 17 +++++++++++++++++ > > > > > 1 file changed, 17 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c > > > > > b/drivers/gpu/drm/i915/intel_fbdev.c > > > > > index df00e6b01f0d..c1a20c3babde 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_fbdev.c > > > > > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > > > > > @@ -290,6 +290,8 @@ static bool intel_fb_initial_config(struct > > > > > drm_fb_helper *fb_helper, > > > > > int i, j; > > > > > bool *save_enabled; > > > > > bool fallback = true; > > > > > + int num_connectors_enabled = 0; > > > > > + int num_connectors_detected = 0; > > > > > > > > > > /* > > > > > * If the user specified any force options, just bail here > > > > > @@ -324,6 +326,10 @@ static bool intel_fb_initial_config(struct > > > > > drm_fb_helper *fb_helper, > > > > > > > > > > fb_conn = fb_helper->connector_info[i]; > > > > > connector = fb_conn->connector; > > > > > + > > > > > + if (connector->status == connector_status_connected) > > > > > + num_connectors_detected++; > > > > > + > > > > > if (!enabled[i]) { > > > > > DRM_DEBUG_KMS("connector %d not enabled, > > > > > skipping\n", > > > > > connector->base.id); > > > > > @@ -338,6 +344,8 @@ static bool intel_fb_initial_config(struct > > > > > drm_fb_helper *fb_helper, > > > > > continue; > > > > > } > > > > > > > > > > + num_connectors_enabled++; > > > > > + > > > > > new_crtc = intel_fb_helper_crtc(fb_helper, > > > > > encoder->crtc); > > > > > > > > > > /* > > > > > @@ -393,6 +401,15 @@ static bool intel_fb_initial_config(struct > > > > > drm_fb_helper *fb_helper, > > > > > fallback = false; > > > > > } > > > > > > > > > > + /* > > > > > + * If the BIOS didn't enable everything it could, fall back to > > > > > have the > > > > > + * same user experiencing of lighting up as much as possible > > > > > like the > > > > > + * fbdev helper library. > > > > > + */ > > > > > + if (num_connectors_enabled != num_connectors_detected && > > > > > + num_connectors_enabled < INTEL_INFO(dev)->num_pipes) > > > > > + fallback = true; > > > > > > > > I think we need a debug message in here so people can figure out why > > > > their fastboot failed with this patch included. E.g. "some connected > > > > outputs weren't enabled, falling back to old behavior". > > > > > > > > Also note that this will probably always happen in certain configs, and > > > > the fallback behavior won't be any better since we may not be able to > > > > light up everything that's attached. > > > > > > Excellent suggestion, I've gone ahead and added debug output for all cases > > > where we fall back. > > > > > > > > With those caveats: > > > > Reviewed-by: Jesse Barnes <jbar...@virtuousgeek.org> > > > > Thinking about this some more last night, I think it would be better to > > count the pipes and the connectors, and bail out if we have detected > > connectors available but not enabled and some free pipes. That would > > prevent unnecessary fastboot breakage I think. > > I do take pipes into account and only bail out if we'd have a free one. > I don't see what more we could do (beside trying to keep the crtcs for the > already enabled connectors)? Ah ok I missed the check against num_pipes... yeah looks like it should be fine. Thanks, -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
