Re: [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable for valleyview platform.

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 18 Jun 2014, "Wang, Quanxian" <quanxian.wang@intel.com> wrote:
>> -----Original Message-----
>> From: Jani Nikula [mailto:jani.nikula@linux.intel.com]
>> Sent: Tuesday, June 17, 2014 11:12 PM
>> To: Wang, Quanxian
>> Cc: intel-gfx@lists.freedesktop.org; Daniel Vetter
>> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable
>> for valleyview platform.
>> 
>> On Tue, 17 Jun 2014, "Wang, Quanxian" <quanxian.wang@intel.com> wrote:
>> >> -----Original Message-----
>> >> From: Jani Nikula [mailto:jani.nikula@linux.intel.com]
>> >> Sent: Monday, June 16, 2014 4:18 PM
>> >> To: Wang, Quanxian; Daniel Vetter
>> >> Cc: intel-gfx@lists.freedesktop.org
>> >> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
>> >> reliable for valleyview platform.
>> >>
>> >> On Mon, 16 Jun 2014, "Wang, Quanxian" <quanxian.wang@intel.com>
>> wrote:
>> >> >> -----Original Message-----
>> >> >> From: Jani Nikula [mailto:jani.nikula@linux.intel.com]
>> >> >> Sent: Friday, June 13, 2014 5:12 PM
>> >> >> To: Daniel Vetter; Wang, Quanxian
>> >> >> Cc: intel-gfx@lists.freedesktop.org
>> >> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is
>> >> >> not reliable for valleyview platform.
>> >> >>
>> >> >> On Fri, 13 Jun 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
>> >> >> > On Fri, Jun 13, 2014 at 02:52:04PM +0800, Quanxian Wang wrote:
>> >> >> >> DP connector will be disconnected after chvt to another console
>> >> >> >> for
>> >> >> >> 10 minutes or more on valleyview platform VTC1010.
>> >> >> >
>> >> >> > This needs _much_ more detail, really.
>> >> >> >
>> >> >> > Also it smells like we work around a sink issue, which means the
>> >> >> > correct quirk is to use some sink id (like OUI), _not_ the platform.
>> >> >> > Since this way you break all DP1.1+ stuff on vlv and if someone
>> >> >> > puts this panel onto a different platform it still doesn't work.
>> >> >>
>> >> >> Furthermore you should end up in this code path *only* if you have
>> >> >> a DP branch device. This shouldn't happen for eDP or native DP
>> >> >> displays. Please confirm what kind of setup you're experiencing
>> >> >> issues
>> >> with.
>> >> >>
>> >> >> Frankly I wouldn't be surpised if we do have issues with branch
>> >> >> devices, but this is not the fix.
>> >> > [Wang, Quanxian] Any idea how to do it? Currently in VTC1010
>> >> > device, we use native DP to connect HDMI monitor.(DP2HDMI) This
>> >> > case will
>> >> happen.
>> >>
>> >> So it's an active adapter?
>> > [Wang, Quanxian] yes.
>> >>
>> >> Please send full dmesg from early booth with drm.debug=0xe module
>> >> parameter set, exhibiting the problem.
>> > [Wang, Quanxian] I will send the dmesg log soon. If open drm.debug=0xe,
>> irq log will overwrite all the dmesg output. I will have some change to get the
>> complete log for you. Just wait for a while.
>> >
>> 
>> What's the irq log you talk about? HPD? That might explain the issues.
> [Wang, Quanxian] right, Too many valleyview_irq_handler log output.
>> 
>> > After checking with hardware spec, I have some comment for registers
>> > of Display Port In i915_reg.h, I found we use PCB_DP_x(address
>> > 0xe4100+??, control, data...) to do the communication and check what the
>> SINK_COUNT. (I found it was defined in Ivybridge spec 2012) The process
>> focus on South Display Engine to do the communication.
>> > But in valleyview spec(2014), I don't find 0xe4110, and only
>> > 0x64100+xxx for north display engine are available. (DPx_AUX_CH_CTL
>> > series defined in i915_reg.h)
>> >
>> > Question: Is the something changed for that after valleyview or
>> haswell(2013-2014)?
>> 
>> If you think we're using PCH registers on VLV/BYT, please point me at the
>> lines of code.
> [Wang, Quanxian] i915/intel_dp.c:3709, with debug, I found it use PCH_DPB_AUX_CH_CTL for VLV. However from the boot information, it seems work. But in VLV bspec, I don't find it. So I am doubt if some hardware process is changed. Sorry I just point that, but I am not clear of display port process for reading status SINK_COUNT.

See the display port spec for details.

Obviously code references require a commit reference as well; the code
changes rapidly. The code I'm looking at has if (HAS_DDI(dev)) right
before the snippet below, i.e. not executed on VLV.

BR,
Jani.


> 3709                 case PORT_B:
> 3710                         intel_dp->aux_ch_ctl_reg = PCH_DPB_AUX_CH_CTL;
> 3711                         break;
> 3712                 case PORT_C:
> 3713                         intel_dp->aux_ch_ctl_reg = PCH_DPC_AUX_CH_CTL;
> 3714                         break;
> 3715                 case PORT_D:
> 3716                         intel_dp->aux_ch_ctl_reg = PCH_DPD_AUX_CH_CTL;
>
> i915/intel_dp.c:3830 (function intel_dp_aux_ch)
> 455                         for (i = 0; i < send_bytes; i += 4)
>  456                                 I915_WRITE(ch_data + i,
>  457                                            pack_aux(send + i, send_bytes - i));
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> >
>> > Thanks
>> >>
>> >> BR,
>> >> Jani.
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >>
>> >> >>
>> >> >>
>> >> >> BR,
>> >> >> Jani.
>> >> >>
>> >> >>
>> >> >> >
>> >> >> > Or I completely don't understand this at all.
>> >> >> >
>> >> >> > Also, such a patch needs a full spec quote or a w/a citation or
>> >> >> > something solid if it's a more generic issue.
>> >> >> > -Daniel
>> >> >> >>
>> >> >> >> Signed-off-by: Quanxian Wang <quanxian.wang@intel.com>
>> >> >> >> ---
>> >> >> >>  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
>> >> >> >>  1 file changed, 3 insertions(+), 1 deletion(-)
>> >> >> >>
>> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> >> >> >> b/drivers/gpu/drm/i915/intel_dp.c index 2688f6d..0d127a5 100644
>> >> >> >> --- a/drivers/gpu/drm/i915/intel_dp.c
>> >> >> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> >> >> >> @@ -2942,6 +2942,7 @@ intel_dp_check_link_status(struct
>> >> >> >> intel_dp
>> >> >> >> *intel_dp)  static enum drm_connector_status
>> >> >> >> intel_dp_detect_dpcd(struct intel_dp *intel_dp)  {
>> >> >> >> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> >> >> >>  	uint8_t *dpcd = intel_dp->dpcd;
>> >> >> >>  	uint8_t type;
>> >> >> >>
>> >> >> >> @@ -2953,7 +2954,8 @@ intel_dp_detect_dpcd(struct intel_dp
>> >> *intel_dp)
>> >> >> >>  		return connector_status_connected;
>> >> >> >>
>> >> >> >>  	/* If we're HPD-aware, SINK_COUNT changes dynamically */
>> >> >> >> -	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
>> >> >> >> +	if (!IS_VALLEYVIEW(dev) &&
>> >> >> >> +	    intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
>> >> >> >>  	    intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) {
>> >> >> >>  		uint8_t reg;
>> >> >> >>  		if (!intel_dp_aux_native_read_retry(intel_dp,
>> >> >> DP_SINK_COUNT,
>> >> >> >> --
>> >> >> >> 1.8.1.2
>> >> >> >>
>> >> >> >> _______________________________________________
>> >> >> >> Intel-gfx mailing list
>> >> >> >> Intel-gfx@lists.freedesktop.org
>> >> >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> >> >
>> >> >> > --
>> >> >> > Daniel Vetter
>> >> >> > Software Engineer, Intel Corporation
>> >> >> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>> >> >> > _______________________________________________
>> >> >> > Intel-gfx mailing list
>> >> >> > Intel-gfx@lists.freedesktop.org
>> >> >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> >>
>> >> >> --
>> >> >> Jani Nikula, Intel Open Source Technology Center
>> >>
>> >> --
>> >> Jani Nikula, Intel Open Source Technology Center
>> 
>> --
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center