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.oliveira@xxxxxxxxx>
> Signed-off-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
