Re: [PATCH] drm/i915/dp: Do not set the eDP link rate/lane count to max

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 08 Mar 2018, Manasi Navare <manasi.d.navare@intel.com> wrote:
> The panels are generally designed to support only a single
> clock and lane configuration, and typically these values
> correspond to the native resolution of the panel. But some
> panels advertise the MAX_LINK_RATE in DPCD higher than what
> is required to support the native resolution.
> So optimize and set the link rate and lane count to the
> least values required by the panel to support the native
> resolution. This will also be an effective power saving
> for such eDP panels.

I'm afraid this will lead to flickering on plenty of laptops. It will
just get reverted when it hits end users. We've been down this path
before. If we decide to try to do this again, we need to figure out how
*not* to regress those machines. We can't just talk our way out of this.

As a tip, it's often useful to have a look at git blame and git log when
you're considering changes to code that seems odd or contentious or
something. In this case that leads to commit 344c5bbcb7a2
("drm/i915/edp: use lane count and link rate from DPCD for eDP"), which
in turn leads to commit 56071a207602 ("drm/i915: use lane count and link
rate from VBT as minimums for eDP") and a bunch of bugzilla links.

That stuff was done by yours truly years ago, so I'd rather not repeat
my mistakes now. ;)


BR,
Jani.

>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Matt Atwood <matthew.s.atwood@intel.com>
> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8d1d7af..ba1114b 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1812,15 +1812,14 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  			bpp = dev_priv->vbt.edp.bpp;
>  		}
>  
> -		/*
> -		 * Use the maximum clock and number of lanes the eDP panel
> -		 * advertizes being capable of. The panels are generally
> -		 * designed to support only a single clock and lane
> -		 * configuration, and typically these values correspond to the
> -		 * native resolution of the panel.
> +		/* The panels are generally designed to support only a single
> +		 * clock and lane configuration, and typically these values
> +		 * correspond to the native resolution of the panel. But some
> +		 * panels advertise higher link rates that might not be required
> +		 * for the native resolution of the panel. So use the least
> +		 * required link rate/lane count for the panel's native
> +		 * resolution.
>  		 */
> -		min_lane_count = max_lane_count;
> -		min_clock = max_clock;
>  	}
>  
>  	for (; bpp >= 6*3; bpp -= 2*3) {

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx