Re: [PATCH 4/4] drm/i915: use current mode if the size matches the preferred mode

Daniel Vetter
2014-10-21T14:53:00Z

On Thu, Oct 09, 2014 at 12:57:45PM -0700, Jesse Barnes wrote:
> From: Kristian Høgsberg <hoegsberg@gmail.com>
>
> The BIOS may set a native mode that doesn't quite match the preferred
> mode timings. It should be ok to use however if it uses the same size,
> so try to avoid a mode set in that case.
>
> Signed-off-by: Kristian Høgsberg <hoegsberg@gmail.com>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Why exactly does this fail? Is the clock off slightly or are the timings
off?

I just wonder whether we should check vrefresh to make sure the bios
doesn't sneak a low refresh rate mode past us (from stuck drrs or
whatever).

/me thinking a bit paranoid again.

-Daniel
> ---
> drivers/gpu/drm/i915/intel_fbdev.c | 47 +++++++++++++++-----------------------
> 1 file changed, 18 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 9b584f3..79dd814 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -398,42 +398,31 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper,
> /* go for command line mode first */
> modes[i] = drm_pick_cmdline_mode(fb_conn, width, height);
>
> - /* try for preferred next */
> + /* try for preferred next or match current */
> if (!modes[i]) {
> - DRM_DEBUG_KMS("looking for preferred mode on connector %s\n",
> - connector->name);
> - modes[i] = drm_has_preferred_mode(fb_conn, width,
> - height);
> - }
> + struct drm_display_mode *preferred;
>
> - /* No preferred mode marked by the EDID? Are there any modes? */
> - if (!modes[i] && !list_empty(&connector->modes)) {
> - DRM_DEBUG_KMS("using first mode listed on connector %s\n",
> + DRM_DEBUG_KMS("looking for preferred mode on connector %s\n",
> connector->name);
> - modes[i] = list_first_entry(&connector->modes,
> - struct drm_display_mode,
> - head);
> - }
> + preferred = drm_has_preferred_mode(fb_conn, width,
> + height);
>
> - /* last resort: use current mode */
> - if (!modes[i]) {
> - /*
> - * IMPORTANT: We want to use the adjusted mode (i.e.
> - * after the panel fitter upscaling) as the initial
> - * config, not the input mode, which is what crtc->mode
> - * usually contains. But since our current fastboot
> - * code puts a mode derived from the post-pfit timings
> - * into crtc->mode this works out correctly. We don't
> - * use hwmode anywhere right now, so use it for this
> - * since the fb helper layer wants a pointer to
> - * something we own.
> - */
> - DRM_DEBUG_KMS("looking for current mode on connector %s\n",
> - connector->name);
> intel_mode_from_pipe_config(&encoder->crtc->hwmode,
> &to_intel_crtc(encoder->crtc)->config);
> - modes[i] = &encoder->crtc->hwmode;
> + modes[i] = &encoder->crtc->hwmode;
> +
> + if (preferred &&
> + !drm_mode_same_size(preferred, modes[i])) {
> + DRM_DEBUG_KMS("using preferred mode %s "
> + "instead of current mode %s "
> + "on connector %d\n",
> + preferred->name,
> + modes[i]->name,
> + fb_conn->connector->base.id);
> + modes[i] = preferred;
> + }
> }
> +
> crtcs[i] = new_crtc;
>
> DRM_DEBUG_KMS("connector %s on pipe %c [CRTC:%d]: %dx%d%s\n",
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch