Re: [Intel-gfx] [PATCH 1/2] drm/i915: Flatten intel_dp_check_mst_status() a bit

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Apr 17, 2020 at 02:51:39PM -0400, Lyude Paul wrote:
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> 

Thanks. Series pushed to dinq.

> On Fri, 2020-04-17 at 18:27 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Make intel_dp_check_mst_status() somewhat legible by humans.
> > 
> > Note that the return value of drm_dp_mst_hpd_irq() is always
> > either 0 or -ENOMEM, and we never did anything with the latter
> > so we can just ignore the whole thing.
> > 
> > We can also get rid of the direct drm_dp_mst_topology_mgr_set_mst(false)
> > call since returning -EINVAL causes the caller to do the very same call
> > for us.
> > 
> > Cc: Lyude Paul <lyude@redhat.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 88 ++++++++++++-------------
> >  1 file changed, 41 insertions(+), 47 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 48397b2c08cf..4d4898db38e9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -5628,61 +5628,55 @@ static int
> >  intel_dp_check_mst_status(struct intel_dp *intel_dp)
> >  {
> >  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > -	bool bret;
> >  
> > -	if (intel_dp->is_mst) {
> > -		u8 esi[DP_DPRX_ESI_LEN] = { 0 };
> > -		int ret = 0;
> > +	if (!intel_dp->is_mst)
> > +		return -EINVAL;
> > +
> > +	WARN_ON_ONCE(intel_dp->active_mst_links < 0);
> > +
> > +	for (;;) {
> > +		u8 esi[DP_DPRX_ESI_LEN] = {};
> > +		bool bret, handled;
> >  		int retry;
> > -		bool handled;
> >  
> > -		WARN_ON_ONCE(intel_dp->active_mst_links < 0);
> >  		bret = intel_dp_get_sink_irq_esi(intel_dp, esi);
> > -go_again:
> > -		if (bret == true) {
> > -
> > -			/* check link status - esi[10] = 0x200c */
> > -			if (intel_dp->active_mst_links > 0 &&
> > -			    !drm_dp_channel_eq_ok(&esi[10], intel_dp-
> > >lane_count)) {
> > -				drm_dbg_kms(&i915->drm,
> > -					    "channel EQ not ok,
> > retraining\n");
> > -				intel_dp_start_link_train(intel_dp);
> > -				intel_dp_stop_link_train(intel_dp);
> > -			}
> > -
> > -			drm_dbg_kms(&i915->drm, "got esi %3ph\n", esi);
> > -			ret = drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi,
> > &handled);
> > -
> > -			if (handled) {
> > -				for (retry = 0; retry < 3; retry++) {
> > -					int wret;
> > -					wret = drm_dp_dpcd_write(&intel_dp-
> > >aux,
> > -								 DP_SINK_COUNT
> > _ESI+1,
> > -								 &esi[1], 3);
> > -					if (wret == 3) {
> > -						break;
> > -					}
> > -				}
> > -
> > -				bret = intel_dp_get_sink_irq_esi(intel_dp,
> > esi);
> > -				if (bret == true) {
> > -					drm_dbg_kms(&i915->drm,
> > -						    "got esi2 %3ph\n", esi);
> > -					goto go_again;
> > -				}
> > -			} else
> > -				ret = 0;
> > -
> > -			return ret;
> > -		} else {
> > +		if (!bret) {
> >  			drm_dbg_kms(&i915->drm,
> >  				    "failed to get ESI - device may have
> > failed\n");
> > -			intel_dp->is_mst = false;
> > -			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
> > -							intel_dp->is_mst);
> > +			return -EINVAL;
> > +		}
> > +
> > +		/* check link status - esi[10] = 0x200c */
> > +		/*
> > +		 * FIXME kill this and use the SST retraining approach
> > +		 * for MST as well.
> > +		 */
> > +		if (intel_dp->active_mst_links > 0 &&
> > +		    !drm_dp_channel_eq_ok(&esi[10], intel_dp->lane_count)) {
> > +			drm_dbg_kms(&i915->drm,
> > +				    "channel EQ not ok, retraining\n");
> > +			intel_dp_start_link_train(intel_dp);
> > +			intel_dp_stop_link_train(intel_dp);
> > +		}
> > +
> > +		drm_dbg_kms(&i915->drm, "got esi %3ph\n", esi);
> > +
> > +		drm_dp_mst_hpd_irq(&intel_dp->mst_mgr, esi, &handled);
> > +		if (!handled)
> > +			break;
> > +
> > +		for (retry = 0; retry < 3; retry++) {
> > +			int wret;
> > +
> > +			wret = drm_dp_dpcd_write(&intel_dp->aux,
> > +						 DP_SINK_COUNT_ESI+1,
> > +						 &esi[1], 3);
> > +			if (wret == 3)
> > +				break;
> >  		}
> >  	}
> > -	return -EINVAL;
> > +
> > +	return 0;
> >  }
> >  
> >  static bool
> > -- 
> > 2.24.1
> > 
> -- 
> Cheers,
> 	Lyude Paul (she/her)
> 	Associate Software Engineer at Red Hat

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx