[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/5] drm/i915: Splitting intel_dp_detect

[Ander Conselvan De Oliveira]

On Thu, 2016-04-07 at 10:58 +0300, Jani Nikula wrote:
> On Wed, 06 Apr 2016, Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com> wrote:
> > On 04/04/16 12:41, Tvrtko Ursulin wrote:
> > > 
> > > On 04/04/16 12:08, Jani Nikula wrote:
> > > > On Mon, 04 Apr 2016, Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> > > > wrote:
> > > > > On 01/04/16 08:41, Ander Conselvan De Oliveira wrote:
> > > > > > On Thu, 2016-03-31 at 12:38 +0000, Patchwork wrote:
> > > > > > > == Series Details ==
> > > > > > > 
> > > > > > > Series: series starting with [1/5] drm/i915: Splitting
> > > > > > > intel_dp_detect
> > > > > > > URL   : https://patchwork.freedesktop.org/series/5044/
> > > > > > > State : success
> > > > > > 
> > > > > > I pushed those to dinq.
> > > > > 
> > > > > This series seems to break eDP detection on BDW RVP.
> > > > 
> > > > I presume this is due to the sink count check. Can you add debug logging
> > > > to print intel_dp->sink_count after it's been read in
> > > > intel_dp_get_dpcd() please?
> > > 
> > > intel_dp->sink_count is zero here. (raw value, before the
> > > DP_GET_SINK_COUNT.)
> > > 
> > > Also, intel_dp_dpcd_read_wake suggests a possibility for reading garbage
> > > with not overly confident wording for the workaround there.
> > > 
> > > > Then the question is, is this just because you have an RVP with who
> > > > knows what panel, or do we have to take into account potentially broken
> > > > panels too? Then I assume the fix would be to to ignore sink count for
> > > > eDP.
> > > 
> > > No idea. :)
> > 
> > I could really use a solution for this. My only development platform is 
> > incapacitated unless I revert this series which, apart from the extra 
> > work when preparing and sending out patches this is taking, including 
> > lost time waiting on CI which I suspect dislikes patches from top of 
> > unknown bases, I think it won't be so easy to continue doing so when the 
> > conflicts start arriving. :(
> 
> Ander, Shubhangi?
> 
> Would something like this be sensible? Tvrtko, can you give it a go?
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index da0c3d29fda8..0890e71db188 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3799,6 +3799,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  	 */
>  	intel_dp->sink_count = DP_GET_SINK_COUNT(intel_dp->sink_count);
>  
> +	if (is_edp(intel_dp))
> +		intel_dp->sink_count = max(intel_dp->sink_count, 1);

I couldn't find anything in the spec that would make SINK_COUNT behave
differently for eDP, but eDP with 0 sinks simply doesn't make sense, so this
seems sensible to me.

Ander
>  	/*
>  	 * SINK_COUNT == 0 and DOWNSTREAM_PORT_PRESENT == 1 implies that
>  	 * a dongle is present but no display. Unless we require to know
> 
> BR,
> Jani.
> 
> 
>