[Intel-gfx] [PATCH] drm/i915: honour forced connector modes

[Chris Wilson]

On Thu, May 15, 2014 at 12:38:01PM -0700, Jesse Barnes wrote:
> On Thu, 15 May 2014 10:55:41 +0100
> Chris Wilson <chris at chris-wilson.co.uk> wrote:
> 
> > In the move over to use BIOS connector configs, we lost the ability to
> > force a specific set of connectors on or off.  Try to remedy that by
> > dropping back to the old behavior if we detect a hard coded connector
> > config that tries to enable a connector (disabling is easy!).
> > 
> > Based on earlier patches by Jesse Barnes.
> > 
> > v2: Remove Jesse's patch
> > 
> > Reported-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Jesse Barnes <jbarnes at virtuousgeek.org>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_fbdev.c |   35 +++++++++++++----------------------
> >  1 file changed, 13 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > index f5821c0..cc2dd5e 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -306,24 +306,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> >  	int num_connectors_enabled = 0;
> >  	int num_connectors_detected = 0;
> >  
> > -	/*
> > -	 * If the user specified any force options, just bail here
> > -	 * and use that config.
> > -	 */
> > -	for (i = 0; i < fb_helper->connector_count; i++) {
> > -		struct drm_fb_helper_connector *fb_conn;
> > -		struct drm_connector *connector;
> > -
> > -		fb_conn = fb_helper->connector_info[i];
> > -		connector = fb_conn->connector;
> > -
> > -		if (!enabled[i])
> > -			continue;
> > -
> > -		if (connector->force != DRM_FORCE_UNSPECIFIED)
> > -			return false;
> > -	}
> > -
> >  	save_enabled = kcalloc(dev->mode_config.num_connector, sizeof(bool),
> >  			       GFP_KERNEL);
> >  	if (!save_enabled)
> > @@ -349,8 +331,18 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> >  			continue;
> >  		}
> >  
> > +		if (connector->force == DRM_FORCE_OFF) {
> > +			DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n",
> > +				      drm_get_connector_name(connector));
> > +			enabled[i] = false;
> > +			continue;
> > +		}
> > +
> >  		encoder = connector->encoder;
> >  		if (!encoder || WARN_ON(!encoder->crtc)) {
> > +			if (connector->force > DRM_FORCE_OFF)
> > +				goto bail;
> > +
> >  			DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n",
> >  				      drm_get_connector_name(connector));
> >  			enabled[i] = false;
> > @@ -369,8 +361,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> >  		for (j = 0; j < fb_helper->connector_count; j++) {
> >  			if (crtcs[j] == new_crtc) {
> >  				DRM_DEBUG_KMS("fallback: cloned configuration\n");
> > -				fallback = true;
> > -				goto out;
> > +				goto bail;
> >  			}
> >  		}
> >  
> > @@ -418,7 +409,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> >  		}
> >  		crtcs[i] = new_crtc;
> >  
> > -		DRM_DEBUG_KMS("connector %s on pipe %d [CRTC:%d]: %dx%d%s\n",
> > +		DRM_DEBUG_KMS("connector %s on pipe %c [CRTC:%d]: %dx%d%s\n",
> >  			      drm_get_connector_name(connector),
> >  			      pipe_name(to_intel_crtc(encoder->crtc)->pipe),
> >  			      encoder->crtc->base.id,
> > @@ -441,8 +432,8 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> >  		fallback = true;
> >  	}
> >  
> > -out:
> >  	if (fallback) {
> > +bail:
> >  		DRM_DEBUG_KMS("Not using firmware configuration\n");
> >  		memcpy(enabled, save_enabled, dev->mode_config.num_connector);
> >  		kfree(save_enabled);
> 
> Could use a few more comments, e.g. the > DRM_FORCE_OFF, but not a
> blocker:
> Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>

Right it could actually do with a

DRM_DEBUG_KMS("connector %s is forced on by the user, but currently"
              " disabled, bailing.\n", drm_get_connector_name(connector));
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre