Re: [Intel-gfx] [PATCH] Revert "drm/i915/display/vrr: Reset VRR capable property on a long hpd"

["Navare, Manasi" <manasi.d.navare@intel.com>]

On Wed, Feb 23, 2022 at 10:29:16AM +0200, Jani Nikula wrote:
> On Wed, 23 Feb 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > This reverts commit 9bc34b4d0f3cb368241684cc5e0445d435dded44.
> >
> > Just oopses on most machines.
> >
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Auch, "ERROR: It appears as if the changes made in Patchwork_22288
> prevented too many machines from booting." [1].

Why would it prevent machines from booting where this only sets the property to false and we tested this
on VRR panels and it works as expected and infact is needed to turn VRR off if disabled from
the panel and also on the connector disconnect.

Our customers already want this.

We should look at whther this prevented the machines from booting or this just revealed some other issue.

> 
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> [1] https://lore.kernel.org/r/164506400291.18053.4904919379639827273@emeril.freedesktop.org
> 
> 
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 17 ++++-------------
> >  1 file changed, 4 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 929e9b6febf1..1046e7fe310a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -4455,12 +4455,6 @@ intel_dp_detect(struct drm_connector *connector,
> >  		memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
> >  		memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
> >  
> > -		/* Reset VRR Capable property */
> > -		drm_dbg_kms(&dev_priv->drm, "[CONNECTOR:%d:%s] VRR capable: FALSE\n",
> > -			    connector->base.id, connector->name);
> > -		drm_connector_set_vrr_capable_property(connector,
> > -						       false);

Do we perhaps need a condition here to reset this only if it was set previously or if it is a VRR panel?

Manasi

> > -
> >  		if (intel_dp->is_mst) {
> >  			drm_dbg_kms(&dev_priv->drm,
> >  				    "MST device may have disappeared %d vs %d\n",
> > @@ -4575,18 +4569,15 @@ static int intel_dp_get_modes(struct drm_connector *connector)
> >  {
> >  	struct intel_connector *intel_connector = to_intel_connector(connector);
> >  	struct edid *edid;
> > -	struct drm_i915_private *i915 = to_i915(connector->dev);
> >  	int num_modes = 0;
> >  
> >  	edid = intel_connector->detect_edid;
> >  	if (edid) {
> > -		bool vrr_capable;
> > -
> >  		num_modes = intel_connector_update_modes(connector, edid);
> > -		vrr_capable = intel_vrr_is_capable(connector);
> > -		drm_dbg_kms(&i915->drm, "[CONNECTOR:%d:%s] VRR capable: %s\n",
> > -			    connector->base.id, connector->name, yesno(vrr_capable));
> > -		drm_connector_set_vrr_capable_property(connector, vrr_capable);
> > +
> > +		if (intel_vrr_is_capable(connector))
> > +			drm_connector_set_vrr_capable_property(connector,
> > +							       true);
> >  	}
> >  
> >  	/* Also add fixed mode, which may or may not be present in EDID */
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center