On 11/11/2014 10:30 PM, Jesse Barnes wrote:
> This should allow us to avoid mode sets for some panel fitter config
> changes.
> 
> v2:
>    - fixup pfit comment (Ander)
> v3:
>    - fixup pfit disable shortcut, only apply to gen4 for now (Jesse)

I see that the patch does what it claims it does, so you can use the 
following r-b tag. However, I want to point out that all the 
documentation I came across, including for PCH platforms, state that 
the proper modeset order is to enable pfit before the pipe is enabled 
and to disable it after disabling the pipe. So I assume you know better 
than the docs.

Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.olive...@intel.com>

> Signed-off-by: Jesse Barnes <jbar...@virtuousgeek.org>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 73 
> ++++++++++++++++++++++++++++++------
>   1 file changed, 62 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 58ed4ef..e7175b1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2861,17 +2861,8 @@ static void intel_update_pipe_size(struct intel_crtc 
> *crtc)
>               return;
>   
>       /*
> -      * Update pipe size and adjust fitter if needed: the reason for this is
> -      * that in compute_mode_changes we check the native mode (not the pfit
> -      * mode) to see if we can flip rather than do a full mode set. In the
> -      * fastboot case, we'll flip, but if we don't update the pipesrc and
> -      * pfit state, we'll end up with a big fb scanned out into the wrong
> -      * sized surface.
> -      *
> -      * To fix this properly, we need to hoist the checks up into
> -      * compute_mode_changes (or above), check the actual pfit state and
> -      * whether the platform allows pfit disable with pipe active, and only
> -      * then update the pipesrc and pfit state, even on the flip path.
> +      * See intel_pfit_changed for info on when we're allowed to
> +      * do this w/o a pipe shutdown.
>        */
>   
>       adjusted_mode = &crtc->config.adjusted_mode;
> @@ -11266,6 +11257,62 @@ static void disable_crtc_nofb(struct intel_crtc 
> *crtc)
>       crtc->new_config = NULL;
>   }
>   
> +/* Do we need a mode set due to pfit changes? */
> +static bool intel_pfit_changed(struct drm_device *dev,
> +                            struct intel_crtc_config *new_config,
> +                            struct intel_crtc_config *cur_config)
> +{
> +     bool ret = false;
> +
> +     if (HAS_DDI(dev) || HAS_PCH_SPLIT(dev)) {
> +             /*
> +              * On PCH platforms we can disable pfit w/o a pipe shutdown,
> +              * otherwise we'll need a mode set.
> +              */
> +             if (new_config->pch_pfit.enabled &&
> +                 cur_config->pch_pfit.enabled)
> +                     ret = false;
> +             else if (new_config->pch_pfit.enabled &&
> +                      !cur_config->pch_pfit.enabled)
> +                     ret = true;
> +             else if (!new_config->pch_pfit.enabled &&
> +                      cur_config->pch_pfit.enabled)
> +                     ret = false;
> +             else if (!new_config->pch_pfit.enabled &&
> +                      !cur_config->pch_pfit.enabled)
> +                     ret = false;
> +     } else if (IS_GEN4(dev)) {
> +             /*
> +              * We may be able to get away with this on more chips, but
> +              * we need more testing here.
> +              */
> +             bool new_enabled, old_enabled;
> +
> +             new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
> +             old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
> +
> +             /* 9xx only needs a shutdown to disable pfit */
> +             if (new_enabled && old_enabled)
> +                     ret = false;
> +             else if (new_enabled && !old_enabled)
> +                     ret = false;
> +             else if (!new_enabled && old_enabled)
> +                     ret = true;
> +             else if (!new_enabled && !old_enabled)
> +                     ret = false;
> +     } else {
> +             bool new_enabled, old_enabled;
> +
> +             new_enabled = !!(new_config->gmch_pfit.control & PFIT_ENABLE);
> +             old_enabled = !!(cur_config->gmch_pfit.control & PFIT_ENABLE);
> +
> +             if (new_enabled != old_enabled)
> +                     ret = true;
> +     }
> +
> +     return ret;
> +}
> +
>   static int intel_crtc_set_config(struct drm_mode_set *set)
>   {
>       struct drm_device *dev;
> @@ -11334,6 +11381,10 @@ static int intel_crtc_set_config(struct drm_mode_set 
> *set)
>               if (to_intel_crtc(set->crtc)->new_config->has_infoframe ||
>                   to_intel_crtc(set->crtc)->config.has_infoframe)
>                       config->mode_changed = true;
> +
> +             if (intel_pfit_changed(dev, 
> to_intel_crtc(set->crtc)->new_config,
> +                                    &to_intel_crtc(set->crtc)->config))
> +                     config->mode_changed = true;
>       }
>   
>       /* set_mode will free it in the mode_changed case */
> 

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

Reply via email to