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

Jani Nikula
2022-08-29T08:57:05Z

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