Re: [Intel-gfx] [PATCH 11/11] drm/i915: Infer vrefresh range for eDP if the EDID omits it

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

On Sat, 27 Aug 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> A bunch of machines seem to have eDP panels where the EDID
> indicates continuous frequency support but fails to actually
> include the range descirptor. This violates the EDID 1.4
> spec, but looks like the Windows driver just hacks around
> this by just assuming that the panel supports a continuous
> refresh rate range that covers all EDID reported modes.
>
> Do the same so that we get VRR support on these machines.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6323
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 45 +++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 8d1559323412..1f3e4824d316 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5207,6 +5207,49 @@ intel_edp_add_properties(struct intel_dp *intel_dp)
>  						       fixed_mode->vdisplay);
>  }
>  
> +/*
> + * Some VRR eDP panels violate the EDID spec and neglect
> + * to include the monitor range descriptor in the EDID.
> + * Cook up the VRR refresh rate limits based on the modes
> + * reported by the panel.
> + */
> +static void
> +intel_edp_infer_vrr_range(struct intel_connector *connector)
> +{
> +	struct drm_i915_private *i915 = to_i915(connector->base.dev);
> +	struct drm_display_info *info = &connector->base.display_info;
> +	const struct edid *edid = connector->edid;
> +	const struct drm_display_mode *mode;
> +
> +	if (!HAS_VRR(i915))
> +		return;
> +
> +	if (!edid || edid->revision < 4 ||

connector->edid is actually an error pointer. I made it so way back when
to differentiate between "broken edid" -EINVAL and "no edid" -ENOENT,
but I don't think we ever use that distinction anywhere.

Either ditch the error pointers (in a separate patch) or check for them
here. With that fixed,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> +	    !(edid->features & DRM_EDID_FEATURE_CONTINUOUS_FREQ) ||
> +	    info->vrr_range.min_vfreq || info->vrr_range.max_vfreq)
> +		return;
> +
> +	if (list_empty(&connector->base.probed_modes))
> +		return;
> +
> +	info->vrr_range.min_vfreq = ~0;
> +	info->vrr_range.max_vfreq = 0;
> +
> +	list_for_each_entry(mode, &connector->base.probed_modes, head) {
> +		int vrefresh = drm_mode_vrefresh(mode);
> +
> +		info->vrr_range.min_vfreq = min_t(int, vrefresh,
> +						  info->vrr_range.min_vfreq);
> +		info->vrr_range.max_vfreq = max_t(int, vrefresh,
> +						  info->vrr_range.max_vfreq);
> +	}
> +
> +	drm_dbg_kms(&i915->drm,
> +		    "[CONNECTOR:%d:%s] does not report refresh rate range, assuming: %d Hz - %d Hz\n",
> +		    connector->base.base.id, connector->base.name,
> +		    info->vrr_range.min_vfreq, info->vrr_range.max_vfreq);
> +}
> +
>  static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  				     struct intel_connector *intel_connector)
>  {
> @@ -5271,6 +5314,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  	}
>  	intel_connector->edid = edid;
>  
> +	intel_edp_infer_vrr_range(intel_connector);
> +
>  	intel_bios_init_panel(dev_priv, &intel_connector->panel,
>  			      encoder->devdata, IS_ERR(edid) ? NULL : edid);

-- 
Jani Nikula, Intel Open Source Graphics Center