On Fri, Nov 09, 2018 at 12:20:10PM -0800, José Roberto de Souza wrote: > Some eDP panels do not set a valid sink count value and even for the > ones that sets is should always be one for eDP, that is why it is not > cached in intel_edp_init_dpcd(). > > But intel_dp_short_pulse() compares the old count with the read one > if there is a mistmatch a full port detection will be executed, what > was happening in the first short pulse interruption of eDP panels > that sets sink count. > > Instead of just skip the compasison for eDP panels, lets not read > the sink count at all for eDP. > > v2: the previous version of this patch was caching the sink count > in intel_edp_init_dpcd() but I was pointed out by Ville a patch that > handled a case of a eDP panel that do not set sink count > > Cc: Ville Syrjälä <ville.syrj...@linux.intel.com> > Cc: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com> > Signed-off-by: José Roberto de Souza <jose.so...@intel.com>
Yeah, I guess we concluded that never reading it is totally fine. Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 44 +++++++++++++++++++-------------- > 1 file changed, 26 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 2b090609bee2..577c166f6483 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3936,8 +3936,6 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp) > static bool > intel_dp_get_dpcd(struct intel_dp *intel_dp) > { > - u8 sink_count; > - > if (!intel_dp_read_dpcd(intel_dp)) > return false; > > @@ -3947,25 +3945,35 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) > intel_dp_set_common_rates(intel_dp); > } > > - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_COUNT, &sink_count) <= 0) > - return false; > - > /* > - * Sink count can change between short pulse hpd hence > - * a member variable in intel_dp will track any changes > - * between short pulse interrupts. > + * Some eDP panels do not set a valid value for sink count, that is why > + * it don't care about read it here and in intel_edp_init_dpcd(). > */ > - intel_dp->sink_count = DP_GET_SINK_COUNT(sink_count); > + if (!intel_dp_is_edp(intel_dp)) { > + u8 count; > + ssize_t r; > > - /* > - * SINK_COUNT == 0 and DOWNSTREAM_PORT_PRESENT == 1 implies that > - * a dongle is present but no display. Unless we require to know > - * if a dongle is present or not, we don't need to update > - * downstream port information. So, an early return here saves > - * time from performing other operations which are not required. > - */ > - if (!intel_dp_is_edp(intel_dp) && !intel_dp->sink_count) > - return false; > + r = drm_dp_dpcd_readb(&intel_dp->aux, DP_SINK_COUNT, &count); > + if (r < 1) > + return false; > + > + /* > + * Sink count can change between short pulse hpd hence > + * a member variable in intel_dp will track any changes > + * between short pulse interrupts. > + */ > + intel_dp->sink_count = DP_GET_SINK_COUNT(count); > + > + /* > + * SINK_COUNT == 0 and DOWNSTREAM_PORT_PRESENT == 1 implies that > + * a dongle is present but no display. Unless we require to know > + * if a dongle is present or not, we don't need to update > + * downstream port information. So, an early return here saves > + * time from performing other operations which are not required. > + */ > + if (!intel_dp->sink_count) > + return false; > + } > > if (!drm_dp_is_branch(intel_dp->dpcd)) > return true; /* native DP sink */ > -- > 2.19.1 -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx