From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 2/2] drm/i915: ignore bios output config if not all outputs are on Date: Tue, 4 Mar 2014 22:08:12 +0100 Message-ID: <20140304210812.GO17001@phenom.ffwll.local> References: <1393963722-5617-1-git-send-email-daniel.vetter@ffwll.ch> <1393963722-5617-2-git-send-email-daniel.vetter@ffwll.ch> <20140304123301.0876a258@jbarnes-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ee0-f45.google.com (mail-ee0-f45.google.com [74.125.83.45]) by gabe.freedesktop.org (Postfix) with ESMTP id A8CCBFAF2C for ; Tue, 4 Mar 2014 13:08:16 -0800 (PST) Received: by mail-ee0-f45.google.com with SMTP id d17so36835eek.18 for ; Tue, 04 Mar 2014 13:08:16 -0800 (PST) Content-Disposition: inline In-Reply-To: <20140304123301.0876a258@jbarnes-desktop> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Jesse Barnes Cc: Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Tue, Mar 04, 2014 at 12:33:01PM -0800, Jesse Barnes wrote: > On Tue, 4 Mar 2014 21:08:42 +0100 > Daniel Vetter 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 > > Cc: Ville Syrj=E4l=E4 > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=3D75552 > > Tested-by: Ville Syrj=E4l=E4 > > Signed-off-by: Daniel Vetter > > --- > > 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_h= elper *fb_helper, > > int i, j; > > bool *save_enabled; > > bool fallback =3D true; > > + int num_connectors_enabled =3D 0; > > + int num_connectors_detected =3D 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 =3D fb_helper->connector_info[i]; > > connector =3D fb_conn->connector; > > + > > + if (connector->status =3D=3D 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_h= elper *fb_helper, > > continue; > > } > > = > > + num_connectors_enabled++; > > + > > new_crtc =3D 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 =3D false; > > } > > = > > + /* > > + * If the BIOS didn't enable everything it could, fall back to have t= he > > + * same user experiencing of lighting up as much as possible like the > > + * fbdev helper library. > > + */ > > + if (num_connectors_enabled !=3D num_connectors_detected && > > + num_connectors_enabled < INTEL_INFO(dev)->num_pipes) > > + fallback =3D 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 Thanks for the review, both patches merged to dinq. -Daniel -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch