Re: [PATCH 3/4] drm/i915: don't force full modeset if primary plane is disabled

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Apr 30, 2014 at 10:07:02AM -0700, Matt Roper wrote:
> In a future patch, we'll allow the primary plane to be disabled by
> userspace via the universal plane API.  If a modeset is requested while
> the primary plane is disabled, crtc->primary->fb will be NULL which
> generally triggers a full modeset (except in fastboot situations).  If
> we detect that the crtc is active, but there's no primary plane fb,
> we should still allow a simple plane update rather than a full modeset
> if the mode isn't actually changing (after re-enabling the primary plane
> of course).
> 
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e1e1239..04bd821 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10136,12 +10136,17 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
>  	if (is_crtc_connector_off(set)) {
>  		config->mode_changed = true;
>  	} else if (set->crtc->primary->fb != set->fb) {
> -		/* If we have no fb then treat it as a full mode set */
> +		/*
> +		 * If we have no fb, we can only flip as long as the crtc is
> +		 * active, otherwise we need a full mode set.  The crtc may
> +		 * be active if we've only disabled the primary plane, or
> +		 * in fastboot situations.
> +		 */
>  		if (set->crtc->primary->fb == NULL) {
>  			struct intel_crtc *intel_crtc =
>  				to_intel_crtc(set->crtc);
>  
> -			if (intel_crtc->active && i915.fastboot) {
> +			if (intel_crtc->active) {
>  				DRM_DEBUG_KMS("crtc has no fb, will flip\n");
>  				config->fb_changed = true;
>  			} else {
> @@ -10380,8 +10385,21 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>  		ret = intel_set_mode(set->crtc, set->mode,
>  				     set->x, set->y, set->fb);
>  	} else if (config->fb_changed) {
> +		struct drm_i915_private *dev_priv = dev->dev_private;
> +		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
> +
>  		intel_crtc_wait_for_pending_flips(set->crtc);
>  
> +		/*
> +		 * We need to make sure the primary plane is re-enabled if it
> +		 * has previously been turned off.
> +		 */
> +		if (!intel_crtc->primary_enabled) {
> +			BUG_ON(!intel_crtc->active);

BUG seems a bit harsh.

> +			intel_enable_primary_hw_plane(dev_priv, intel_crtc->plane,
> +						      intel_crtc->pipe);
> +		}
> +
>  		ret = intel_pipe_set_base(set->crtc,
>  					  set->x, set->y, set->fb);

Enable the plane after set_base to avoid lighting up the plane if
set_base fails, and it'll also make the flip+enable atomic and so
we avoid flashing the old fb at the user.

>  		/*
> -- 
> 1.8.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC