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

From: Daniel Vetter <daniel@ffwll.ch>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 4/5] drm/i915: use current mode if the size matches the preferred mode
Date: Tue, 10 Jun 2014 16:05:36 +0200	[thread overview]
Message-ID: <20140610140536.GQ5821@phenom.ffwll.local> (raw)
In-Reply-To: <1401992671-2548-4-git-send-email-jbarnes@virtuousgeek.org>

On Thu, Jun 05, 2014 at 11:24:30AM -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>

Not sure we want this since this seems to override the cmdline options to
force a specific edid. Also not sure whether we shouldn't just add this as
the preferred mode when probing (before the preferred mode the vbt/edid
provides ofc).

What exactly is the mismatch here? It could be DRRS or something fancy,
too.

Not sure what to do here really.
-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 088fe93..09819ae 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -379,42 +379,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 %d [CRTC:%d]: %dx%d%s\n",
> -- 
> 1.8.3.2
> 
> _______________________________________________
> 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