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

Ville Syrjälä
2014-05-15T14:54:31Z

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