On Sun, Jul 21, 2013 at 4:37 PM, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
> In the old days of the crtc helpers we've only had the encoder and
> crtc ->mode_fixup callbacks. So when the lvds connector wanted to
> adjust the crtc timings it had to set a driver-private mode flag to
> tell the crtc mode fixup code to not overwrite them with the generic
> ones.
>
> When converting things to the new infrastructure I've kept the entire
> logic and only moved the flag to pipe_config->timings_set. But this
> logic is pretty tricky and already caused regressions:
>
> commit 21d8a4756af5fdf4a42e79a77cf3b6f52678d443
> Author: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Date: Fri Jul 12 08:07:30 2013 +0200
>
> drm/i915: fix pfit regression for non-autoscaled resolutions
>
> So take advantage of the flexibility our own modeset infrastructure
> affords us and prefill default crtc timings. This allows us to rip out
> ->timings_set. Note that we overwrite things again when retrying the
> pipe config computation due to bandwidth constraints to avoid bogus
> crtc timings if the encoder only does relative adjustments (which is
> how the pfit code works). Only a theoretical concern though since
> platforms where we retry (pch-split platforms) do not need
> adjustements (since only the old gmch pfit needs that). But let's
> better be safe than sorry.
>
> Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 9 +++------
> drivers/gpu/drm/i915/intel_drv.h | 4 ----
> drivers/gpu/drm/i915/intel_panel.c | 3 ---
> 3 files changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b07f891e..aebdadc 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4108,12 +4108,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
> return -EINVAL;
> }
>
> - /* All interlaced capable intel hw wants timings in frames. Note though
> - * that intel_lvds_compute_config does some funny tricks with the crtc
> - * timings, so we need to be careful not to clobber these.*/
> - if (!pipe_config->timings_set)
> - drm_mode_set_crtcinfo(adjusted_mode, 0);
> -
> /* Cantiga+ cannot handle modes with a hsync front porch of 0.
> * WaPruneModeWithIncorrectHsyncOffset:ctg,elk,ilk,snb,ivb,vlv,hsw.
> */
> @@ -7882,6 +7876,9 @@ encoder_retry:
> pipe_config->port_clock = 0;
> pipe_config->pixel_multiplier = 1;
>
> + /* Fill in default crtc timings, allow encoders to overwrite them. */
> + drm_mode_set_crtcinfo(&pipe_config->adjusted_mode, 0);
> +
> /* Pass our mode to the connectors and the CRTC to give them a chance to
> * adjust it according to limitations or connector properties, and also
> * a chance to reject the mode entirely.
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 31087ff..a9eca0e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -208,10 +208,6 @@ struct intel_crtc_config {
>
> struct drm_display_mode requested_mode;
> struct drm_display_mode adjusted_mode;
> - /* This flag must be set by the encoder's compute_config callback if it
> - * changes the crtc timings in the mode to prevent the crtc fixup from
> - * overwriting them. Currently only lvds needs that. */
> - bool timings_set;
> /* Whether to set up the PCH/FDI. Note that we never allow sharing
> * between pch encoders and cpu encoders. */
> bool has_pch_encoder;
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 67e2c1f..01b5a51 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -194,9 +194,6 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
> adjusted_mode->vdisplay == mode->vdisplay)
> goto out;
>
> - drm_mode_set_crtcinfo(adjusted_mode, 0);
I didn't get why this set here isn't needed anymore..
> - pipe_config->timings_set = true;
> -
> switch (fitting_mode) {
> case DRM_MODE_SCALE_CENTER:
> /*
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
