drm/i915/vlv: DP_SINK_COUNT is not reliable for valleyview platform.

Commit Message

Wang, Quanxian June 13, 2014, 6:52 a.m. UTC

DP connector will be disconnected after chvt to another console
for 10 minutes or more on valleyview platform VTC1010.

Signed-off-by: Quanxian Wang <[e-mail]>
---
 drivers/gpu/drm/i915/intel_dp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Chris Wilson June 13, 2014, 6:55 a.m. UTC | #1

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.
> 
> Signed-off-by: Quanxian Wang <[e-mail]>
> ---
>  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) {

  static bool intel_dp_supports_hpd(struct intel_dp *intel_dp)
  {
    if (IS_VALLEYVIEW(intel_dp_to_dev(intel_dp))
       /* SINK_COUNT is unreliable resulting in premature disconnects */
       return false; 

    if (intel_dp->dpcd[DP_DPCD_REV] < 0x11)
      return false;

    return intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
  }

would have been a much preferable patch. Please always consider the next
person to read the code and try to figure out the random predicates.

Is the issue transient? Is there a way to keep the hpd but do a
automatic verification?
-Chris

Daniel Vetter June 13, 2014, 7:16 a.m. UTC | #2

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.

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 <[e-mail]>
> ---
>  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
> [e-mail]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Jani Nikula June 13, 2014, 9:12 a.m. UTC | #3

On Fri, 13 Jun 2014, Daniel Vetter <[e-mail]> 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.


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 <[e-mail]>
>> ---
>>  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
>> [e-mail]
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> [phone] - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> [e-mail]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Wang, Quanxian June 13, 2014, 9:23 a.m. UTC | #4

> -----Original Message-----
> From: Daniel Vetter [mailto:[e-mail]] On Behalf Of Daniel
> Vetter
> Sent: Friday, June 13, 2014 3:16 PM
> To: Wang, Quanxian
> Cc: [e-mail]
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
> reliable for valleyview platform.
> 
> 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.
[Wang, Quanxian] Sorry, I am not sure what detail should be provided.
The background is  in tizen-ivi, when Weston is running on vt1, and chvt to vt2, after 10 or more minutes, Weston will exit.
After the investigation, I found in intel_dp_detect_dpcd function, it will use DP_SINK_COUNT to check if connector is connected or not.  KMS think it is not connected and return connector_status_disconnected, this will disable the DP connector. Weston will not get any disconnector, it will exit. That is the root cause. So I think use DP_SINK_COUNT is not reliable to check connector is connected or not.
> 
> 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.
[Wang, Quanxian] Yes, it is a special issue happens on valleyview platform VTC1010. I just have only one type of valleyview platform vtc1010 and not sure if others vlv platform has the same issue which has dp1.1.
> 
> 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.
[Wang, Quanxian] I am checking the related registers. Thanks.
> -Daniel
> >
> > Signed-off-by: Quanxian Wang <[e-mail]>
> > ---
> >  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
> > [e-mail]
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> [phone] - http://blog.ffwll.ch

Jani Nikula June 13, 2014, 10:17 a.m. UTC | #5

On Fri, 13 Jun 2014, "Wang, Quanxian" <[e-mail]> wrote:
>> -----Original Message-----
>> From: Daniel Vetter [mailto:[e-mail]] On Behalf Of Daniel
>> Vetter
>> Sent: Friday, June 13, 2014 3:16 PM
>> To: Wang, Quanxian
>> Cc: [e-mail]
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
>> reliable for valleyview platform.
>> 
>> 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.
> [Wang, Quanxian] Sorry, I am not sure what detail should be provided.
> The background is  in tizen-ivi, when Weston is running on vt1, and chvt to vt2, after 10 or more minutes, Weston will exit.
> After the investigation, I found in intel_dp_detect_dpcd function, it will use DP_SINK_COUNT to check if connector is connected or not.  KMS think it is not connected and return connector_status_disconnected, this will disable the DP connector. Weston will not get any disconnector, it will exit. That is the root cause. So I think use DP_SINK_COUNT is not reliable to check connector is connected or not.

Please see my other mail, and provide the info.

BR,
Jani.

>> 
>> 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.
> [Wang, Quanxian] Yes, it is a special issue happens on valleyview platform VTC1010. I just have only one type of valleyview platform vtc1010 and not sure if others vlv platform has the same issue which has dp1.1.
>> 
>> 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.
> [Wang, Quanxian] I am checking the related registers. Thanks.
>> -Daniel
>> >
>> > Signed-off-by: Quanxian Wang <[e-mail]>
>> > ---
>> >  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
>> > [e-mail]
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> [phone] - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> [e-mail]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Wang, Quanxian June 16, 2014, 1:54 a.m. UTC | #6

Thanks for your comment.

> -----Original Message-----
> From: Chris Wilson [mailto:[e-mail]]
> Sent: Friday, June 13, 2014 2:55 PM
> To: Wang, Quanxian
> Cc: [e-mail]
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
> reliable for valleyview platform.
> 
> 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.
> >
> > Signed-off-by: Quanxian Wang <[e-mail]>
> > ---
> >  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) {
> 
>   static bool intel_dp_supports_hpd(struct intel_dp *intel_dp)
>   {
>     if (IS_VALLEYVIEW(intel_dp_to_dev(intel_dp))
>        /* SINK_COUNT is unreliable resulting in premature disconnects */
>        return false;
> 
>     if (intel_dp->dpcd[DP_DPCD_REV] < 0x11)
>       return false;
> 
>     return intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
>   }
> 
> would have been a much preferable patch. Please always consider the next
> person to read the code and try to figure out the random predicates.
[Wang, Quanxian] Thanks
> 
> Is the issue transient? Is there a way to keep the hpd but do a automatic
> verification?
[Wang, Quanxian] If the issue happens, from my testing, it will always be there. (Could not start Weston because of no connector is available)
Not found the way. Still need more spec to make sure what is hardware process. Sorry about that.
> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre

Wang, Quanxian June 16, 2014, 1:57 a.m. UTC | #7

> -----Original Message-----
> From: Jani Nikula [mailto:[e-mail]]
> Sent: Friday, June 13, 2014 5:12 PM
> To: Daniel Vetter; Wang, Quanxian
> Cc: [e-mail]
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
> reliable for valleyview platform.
> 
> On Fri, 13 Jun 2014, Daniel Vetter <[e-mail]> 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.
> 
> 
> 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 <[e-mail]>
> >> ---
> >>  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
> >> [e-mail]
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > [phone] - http://blog.ffwll.ch
> > _______________________________________________
> > Intel-gfx mailing list
> > [e-mail]
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Jani Nikula, Intel Open Source Technology Center

Jani Nikula June 16, 2014, 8:18 a.m. UTC | #8

On Mon, 16 Jun 2014, "Wang, Quanxian" <[e-mail]> wrote:
>> -----Original Message-----
>> From: Jani Nikula [mailto:[e-mail]]
>> Sent: Friday, June 13, 2014 5:12 PM
>> To: Daniel Vetter; Wang, Quanxian
>> Cc: [e-mail]
>> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
>> reliable for valleyview platform.
>> 
>> On Fri, 13 Jun 2014, Daniel Vetter <[e-mail]> 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?

Please send full dmesg from early booth with drm.debug=0xe module
parameter set, exhibiting the problem.

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 <[e-mail]>
>> >> ---
>> >>  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
>> >> [e-mail]
>> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> > --
>> > Daniel Vetter
>> > Software Engineer, Intel Corporation
>> > [phone] - http://blog.ffwll.ch
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > [e-mail]
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> --
>> Jani Nikula, Intel Open Source Technology Center

Wang, Quanxian June 17, 2014, 2:14 a.m. UTC | #9

> -----Original Message-----
> From: Jani Nikula [mailto:[e-mail]]
> Sent: Monday, June 16, 2014 4:18 PM
> To: Wang, Quanxian; Daniel Vetter
> Cc: [e-mail]
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable
> for valleyview platform.
> 
> On Mon, 16 Jun 2014, "Wang, Quanxian" <[e-mail]> wrote:
> >> -----Original Message-----
> >> From: Jani Nikula [mailto:[e-mail]]
> >> Sent: Friday, June 13, 2014 5:12 PM
> >> To: Daniel Vetter; Wang, Quanxian
> >> Cc: [e-mail]
> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
> >> reliable for valleyview platform.
> >>
> >> On Fri, 13 Jun 2014, Daniel Vetter <[e-mail]> 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.

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)? 

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 <[e-mail]>
> >> >> ---
> >> >>  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
> >> >> [e-mail]
> >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >
> >> > --
> >> > Daniel Vetter
> >> > Software Engineer, Intel Corporation
> >> > [phone] - http://blog.ffwll.ch
> >> > _______________________________________________
> >> > Intel-gfx mailing list
> >> > [e-mail]
> >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>
> >> --
> >> Jani Nikula, Intel Open Source Technology Center
> 
> --
> Jani Nikula, Intel Open Source Technology Center

Wang, Quanxian June 17, 2014, 6:24 a.m. UTC | #10

File dmesg_normal_20140617.log will contain dmesg log when boot the machine and start weston. (Previous is overwrite, but it is enough for graphics boot message)
File dmesg_error_20140617.log contains dmesg log after Weston exit when it found no connector available.

I disable log for  hotplug event from valleyview_irq_handler. There are so many. Maybe you can find some private debug log. Don't need to care that.

Thanks

Regards

Quanxian Wang


> -----Original Message-----
> From: Wang, Quanxian
> Sent: Tuesday, June 17, 2014 10:14 AM
> To: 'Jani Nikula'
> Cc: [e-mail]; Daniel Vetter
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable
> for valleyview platform.
> 
> 
> 
> > -----Original Message-----
> > From: Jani Nikula [mailto:[e-mail]]
> > Sent: Monday, June 16, 2014 4:18 PM
> > To: Wang, Quanxian; Daniel Vetter
> > Cc: [e-mail]
> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
> > reliable for valleyview platform.
> >
> > On Mon, 16 Jun 2014, "Wang, Quanxian" <[e-mail]>
> wrote:
> > >> -----Original Message-----
> > >> From: Jani Nikula [mailto:[e-mail]]
> > >> Sent: Friday, June 13, 2014 5:12 PM
> > >> To: Daniel Vetter; Wang, Quanxian
> > >> Cc: [e-mail]
> > >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
> > >> reliable for valleyview platform.
> > >>
> > >> On Fri, 13 Jun 2014, Daniel Vetter <[e-mail]> 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.
> 
> 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)?
> 
> 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 <[e-mail]>
> > >> >> ---
> > >> >>  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
> > >> >> [e-mail]
> > >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >> >
> > >> > --
> > >> > Daniel Vetter
> > >> > Software Engineer, Intel Corporation
> > >> > [phone] - http://blog.ffwll.ch
> > >> > _______________________________________________
> > >> > Intel-gfx mailing list
> > >> > [e-mail]
> > >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > >>
> > >> --
> > >> Jani Nikula, Intel Open Source Technology Center
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center

Jani Nikula June 17, 2014, 3:12 p.m. UTC | #11

On Tue, 17 Jun 2014, "Wang, Quanxian" <[e-mail]> wrote:
>> -----Original Message-----
>> From: Jani Nikula [mailto:[e-mail]]
>> Sent: Monday, June 16, 2014 4:18 PM
>> To: Wang, Quanxian; Daniel Vetter
>> Cc: [e-mail]
>> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable
>> for valleyview platform.
>> 
>> On Mon, 16 Jun 2014, "Wang, Quanxian" <[e-mail]> wrote:
>> >> -----Original Message-----
>> >> From: Jani Nikula [mailto:[e-mail]]
>> >> Sent: Friday, June 13, 2014 5:12 PM
>> >> To: Daniel Vetter; Wang, Quanxian
>> >> Cc: [e-mail]
>> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
>> >> reliable for valleyview platform.
>> >>
>> >> On Fri, 13 Jun 2014, Daniel Vetter <[e-mail]> 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.

> 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.

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 <[e-mail]>
>> >> >> ---
>> >> >>  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
>> >> >> [e-mail]
>> >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> >
>> >> > --
>> >> > Daniel Vetter
>> >> > Software Engineer, Intel Corporation
>> >> > [phone] - http://blog.ffwll.ch
>> >> > _______________________________________________
>> >> > Intel-gfx mailing list
>> >> > [e-mail]
>> >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >>
>> >> --
>> >> Jani Nikula, Intel Open Source Technology Center
>> 
>> --
>> Jani Nikula, Intel Open Source Technology Center

Jani Nikula June 17, 2014, 3:38 p.m. UTC | #12

On Tue, 17 Jun 2014, "Wang, Quanxian" <[e-mail]> wrote:
> File dmesg_normal_20140617.log will contain dmesg log when boot the machine and start weston. (Previous is overwrite, but it is enough for graphics boot message)
> File dmesg_error_20140617.log contains dmesg log after Weston exit when it found no connector available.
>
> I disable log for  hotplug event from valleyview_irq_handler. There are so many. Maybe you can find some private debug log. Don't need to care that.

Per DP spec we need to read the SINK_COUNT. Perhaps your problem is the
hotplug irqs?

BR,
Jani.



>
> Thanks
>
> Regards
>
> Quanxian Wang
>
>
>> -----Original Message-----
>> From: Wang, Quanxian
>> Sent: Tuesday, June 17, 2014 10:14 AM
>> To: 'Jani Nikula'
>> Cc: [e-mail]; Daniel Vetter
>> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable
>> for valleyview platform.
>> 
>> 
>> 
>> > -----Original Message-----
>> > From: Jani Nikula [mailto:[e-mail]]
>> > Sent: Monday, June 16, 2014 4:18 PM
>> > To: Wang, Quanxian; Daniel Vetter
>> > Cc: [e-mail]
>> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
>> > reliable for valleyview platform.
>> >
>> > On Mon, 16 Jun 2014, "Wang, Quanxian" <[e-mail]>
>> wrote:
>> > >> -----Original Message-----
>> > >> From: Jani Nikula [mailto:[e-mail]]
>> > >> Sent: Friday, June 13, 2014 5:12 PM
>> > >> To: Daniel Vetter; Wang, Quanxian
>> > >> Cc: [e-mail]
>> > >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
>> > >> reliable for valleyview platform.
>> > >>
>> > >> On Fri, 13 Jun 2014, Daniel Vetter <[e-mail]> 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.
>> 
>> 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)?
>> 
>> 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 <[e-mail]>
>> > >> >> ---
>> > >> >>  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
>> > >> >> [e-mail]
>> > >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> > >> >
>> > >> > --
>> > >> > Daniel Vetter
>> > >> > Software Engineer, Intel Corporation
>> > >> > [phone] - http://blog.ffwll.ch
>> > >> > _______________________________________________
>> > >> > Intel-gfx mailing list
>> > >> > [e-mail]
>> > >> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> > >>
>> > >> --
>> > >> Jani Nikula, Intel Open Source Technology Center
>> >
>> > --
>> > Jani Nikula, Intel Open Source Technology Center

Wang, Quanxian June 18, 2014, 3:14 a.m. UTC | #13

> -----Original Message-----
> From: Jani Nikula [mailto:[e-mail]]
> Sent: Tuesday, June 17, 2014 11:12 PM
> To: Wang, Quanxian
> Cc: [e-mail]; 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" <[e-mail]> wrote:
> >> -----Original Message-----
> >> From: Jani Nikula [mailto:[e-mail]]
> >> Sent: Monday, June 16, 2014 4:18 PM
> >> To: Wang, Quanxian; Daniel Vetter
> >> Cc: [e-mail]
> >> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
> >> reliable for valleyview platform.
> >>
> >> On Mon, 16 Jun 2014, "Wang, Quanxian" <[e-mail]>
> wrote:
> >> >> -----Original Message-----
> >> >> From: Jani Nikula [mailto:[e-mail]]
> >> >> Sent: Friday, June 13, 2014 5:12 PM
> >> >> To: Daniel Vetter; Wang, Quanxian
> >> >> Cc: [e-mail]
> >> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is
> >> >> not reliable for valleyview platform.
> >> >>
> >> >> On Fri, 13 Jun 2014, Daniel Vetter <[e-mail]> 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.
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 <[e-mail]>
> >> >> >> ---
> >> >> >>  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
> >> >> >> [e-mail]
> >> >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >> >
> >> >> > --
> >> >> > Daniel Vetter
> >> >> > Software Engineer, Intel Corporation
> >> >> > [phone] - http://blog.ffwll.ch
> >> >> > _______________________________________________
> >> >> > Intel-gfx mailing list
> >> >> > [e-mail]
> >> >> > 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 June 18, 2014, 10:45 a.m. UTC | #14

On Wed, 18 Jun 2014, "Wang, Quanxian" <[e-mail]> wrote:
>> -----Original Message-----
>> From: Jani Nikula [mailto:[e-mail]]
>> Sent: Tuesday, June 17, 2014 11:12 PM
>> To: Wang, Quanxian
>> Cc: [e-mail]; 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" <[e-mail]> wrote:
>> >> -----Original Message-----
>> >> From: Jani Nikula [mailto:[e-mail]]
>> >> Sent: Monday, June 16, 2014 4:18 PM
>> >> To: Wang, Quanxian; Daniel Vetter
>> >> Cc: [e-mail]
>> >> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
>> >> reliable for valleyview platform.
>> >>
>> >> On Mon, 16 Jun 2014, "Wang, Quanxian" <[e-mail]>
>> wrote:
>> >> >> -----Original Message-----
>> >> >> From: Jani Nikula [mailto:[e-mail]]
>> >> >> Sent: Friday, June 13, 2014 5:12 PM
>> >> >> To: Daniel Vetter; Wang, Quanxian
>> >> >> Cc: [e-mail]
>> >> >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is
>> >> >> not reliable for valleyview platform.
>> >> >>
>> >> >> On Fri, 13 Jun 2014, Daniel Vetter <[e-mail]> 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 <[e-mail]>
>> >> >> >> ---
>> >> >> >>  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
>> >> >> >> [e-mail]
>> >> >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> >> >
>> >> >> > --
>> >> >> > Daniel Vetter
>> >> >> > Software Engineer, Intel Corporation
>> >> >> > [phone] - http://blog.ffwll.ch
>> >> >> > _______________________________________________
>> >> >> > Intel-gfx mailing list
>> >> >> > [e-mail]
>> >> >> > 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

Wang, Quanxian June 23, 2014, 10:37 a.m. UTC | #15

> -----Original Message-----
> From: Jani Nikula [mailto:[e-mail]]
> Sent: Tuesday, June 17, 2014 11:38 PM
> To: Wang, Quanxian
> Cc: [e-mail]; 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" <[e-mail]> wrote:
> > File dmesg_normal_20140617.log will contain dmesg log when boot the
> > machine and start weston. (Previous is overwrite, but it is enough for
> graphics boot message) File dmesg_error_20140617.log contains dmesg log
> after Weston exit when it found no connector available.
> >
> > I disable log for  hotplug event from valleyview_irq_handler. There are so
> many. Maybe you can find some private debug log. Don't need to care that.
> 
> Per DP spec we need to read the SINK_COUNT. Perhaps your problem is the
> hotplug irqs?
[Wang, Quanxian] Hi, Jani
The log event is about the transaction event instead of hotplug event. It is general since they will use I2c to read byte one by one. It will generate many transaction.

But the root cause is really about hotplug(intel_hpd_irq_handler). When we switch to console, there will be a hotplug event happens after a while. After that, the system will continually get the hotplug event to switch sink device on and off periodly.  With DisplayPort 1.2 spec, '' This bit is used for either monitor hotplug/unplug or for notification of a sink event.", I am not sure if it is notification of  a sink event or real hotplug event. We have no code to identify the difference between hotplug/unplug  and notification of a sink event. I check display port spec and also not found how to identify them differently.

Question is: 
In function intel_dp_detect_dpcd, before checking SINK_COUNT, we will use intel_dp_get_dpcd to get the dpcd. Could we think there is an active sink device attached to branch device if dpcd content is not null.
According to the display port spec, only sink device has dpcd, if we get one, there must be a sink device attached to branch device or source device. Right?

> 
> BR,
> Jani.
> 
> 
> 
> >
> > Thanks
> >
> > Regards
> >
> > Quanxian Wang
> >
> >
> >> -----Original Message-----
> >> From: Wang, Quanxian
> >> Sent: Tuesday, June 17, 2014 10:14 AM
> >> To: 'Jani Nikula'
> >> Cc: [e-mail]; Daniel Vetter
> >> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
> >> reliable for valleyview platform.
> >>
> >>
> >>
> >> > -----Original Message-----
> >> > From: Jani Nikula [mailto:[e-mail]]
> >> > Sent: Monday, June 16, 2014 4:18 PM
> >> > To: Wang, Quanxian; Daniel Vetter
> >> > Cc: [e-mail]
> >> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
> >> > reliable for valleyview platform.
> >> >
> >> > On Mon, 16 Jun 2014, "Wang, Quanxian" <[e-mail]>
> >> wrote:
> >> > >> -----Original Message-----
> >> > >> From: Jani Nikula [mailto:[e-mail]]
> >> > >> Sent: Friday, June 13, 2014 5:12 PM
> >> > >> To: Daniel Vetter; Wang, Quanxian
> >> > >> Cc: [e-mail]
> >> > >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is
> >> > >> not reliable for valleyview platform.
> >> > >>
> >> > >> On Fri, 13 Jun 2014, Daniel Vetter <[e-mail]> 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.
> >>
> >> 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)?
> >>
> >> 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 <[e-mail]>
> >> > >> >> ---
> >> > >> >>  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
> >> > >> >> [e-mail]
> >> > >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> > >> >
> >> > >> > --
> >> > >> > Daniel Vetter
> >> > >> > Software Engineer, Intel Corporation
> >> > >> > [phone] - http://blog.ffwll.ch
> >> > >> > _______________________________________________
> >> > >> > Intel-gfx mailing list
> >> > >> > [e-mail]
> >> > >> > 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 June 24, 2014, 3:58 p.m. UTC | #16

On Mon, 23 Jun 2014, "Wang, Quanxian" <[e-mail]> wrote:
>> -----Original Message-----
>> From: Jani Nikula [mailto:[e-mail]]
>> Sent: Tuesday, June 17, 2014 11:38 PM
>> To: Wang, Quanxian
>> Cc: [e-mail]; 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" <[e-mail]> wrote:
>> > File dmesg_normal_20140617.log will contain dmesg log when boot the
>> > machine and start weston. (Previous is overwrite, but it is enough for
>> graphics boot message) File dmesg_error_20140617.log contains dmesg log
>> after Weston exit when it found no connector available.
>> >
>> > I disable log for  hotplug event from valleyview_irq_handler. There are so
>> many. Maybe you can find some private debug log. Don't need to care that.
>> 
>> Per DP spec we need to read the SINK_COUNT. Perhaps your problem is the
>> hotplug irqs?
> [Wang, Quanxian] Hi, Jani
> The log event is about the transaction event instead of hotplug event. It is general since they will use I2c to read byte one by one. It will generate many transaction.
>
> But the root cause is really about hotplug(intel_hpd_irq_handler). When we switch to console, there will be a hotplug event happens after a while. After that, the system will continually get the hotplug event to switch sink device on and off periodly.  With DisplayPort 1.2 spec, '' This bit is used for either monitor hotplug/unplug or for notification of a sink event.", I am not sure if it is notification of  a sink event or real hotplug event. We have no code to identify the difference between hotplug/unplug  and notification of a sink event. I check display port spec and also not found how to identify them differently.
>
> Question is: 
> In function intel_dp_detect_dpcd, before checking SINK_COUNT, we will use intel_dp_get_dpcd to get the dpcd. Could we think there is an active sink device attached to branch device if dpcd content is not null.
> According to the display port spec, only sink device has dpcd, if we get one, there must be a sink device attached to branch device or source device. Right?

No. From your logs, the DPCD has bit 0 set in address 5h, which means
downstream port present, which is only allowed in branch devices.

BR,
Jani.




>
>> 
>> BR,
>> Jani.
>> 
>> 
>> 
>> >
>> > Thanks
>> >
>> > Regards
>> >
>> > Quanxian Wang
>> >
>> >
>> >> -----Original Message-----
>> >> From: Wang, Quanxian
>> >> Sent: Tuesday, June 17, 2014 10:14 AM
>> >> To: 'Jani Nikula'
>> >> Cc: [e-mail]; Daniel Vetter
>> >> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
>> >> reliable for valleyview platform.
>> >>
>> >>
>> >>
>> >> > -----Original Message-----
>> >> > From: Jani Nikula [mailto:[e-mail]]
>> >> > Sent: Monday, June 16, 2014 4:18 PM
>> >> > To: Wang, Quanxian; Daniel Vetter
>> >> > Cc: [e-mail]
>> >> > Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
>> >> > reliable for valleyview platform.
>> >> >
>> >> > On Mon, 16 Jun 2014, "Wang, Quanxian" <[e-mail]>
>> >> wrote:
>> >> > >> -----Original Message-----
>> >> > >> From: Jani Nikula [mailto:[e-mail]]
>> >> > >> Sent: Friday, June 13, 2014 5:12 PM
>> >> > >> To: Daniel Vetter; Wang, Quanxian
>> >> > >> Cc: [e-mail]
>> >> > >> Subject: Re: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is
>> >> > >> not reliable for valleyview platform.
>> >> > >>
>> >> > >> On Fri, 13 Jun 2014, Daniel Vetter <[e-mail]> 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.
>> >>
>> >> 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)?
>> >>
>> >> 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 <[e-mail]>
>> >> > >> >> ---
>> >> > >> >>  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
>> >> > >> >> [e-mail]
>> >> > >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> > >> >
>> >> > >> > --
>> >> > >> > Daniel Vetter
>> >> > >> > Software Engineer, Intel Corporation
>> >> > >> > [phone] - http://blog.ffwll.ch
>> >> > >> > _______________________________________________
>> >> > >> > Intel-gfx mailing list
>> >> > >> > [e-mail]
>> >> > >> > 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

Dave Airlie June 24, 2014, 9:27 p.m. UTC | #17

On 23 June 2014 20:37, Wang, Quanxian <[e-mail]> wrote:
>
>
>> -----Original Message-----
>> From: Jani Nikula [mailto:[e-mail]]
>> Sent: Tuesday, June 17, 2014 11:38 PM
>> To: Wang, Quanxian
>> Cc: [e-mail]; 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" <[e-mail]> wrote:
>> > File dmesg_normal_20140617.log will contain dmesg log when boot the
>> > machine and start weston. (Previous is overwrite, but it is enough for
>> graphics boot message) File dmesg_error_20140617.log contains dmesg log
>> after Weston exit when it found no connector available.
>> >
>> > I disable log for  hotplug event from valleyview_irq_handler. There are so
>> many. Maybe you can find some private debug log. Don't need to care that.
>>
>> Per DP spec we need to read the SINK_COUNT. Perhaps your problem is the
>> hotplug irqs?
> [Wang, Quanxian] Hi, Jani
> The log event is about the transaction event instead of hotplug event. It is general since they will use I2c to read byte one by one. It will generate many transaction.
>
> But the root cause is really about hotplug(intel_hpd_irq_handler). When we switch to console, there will be a hotplug event happens after a while. After that, the system will continually get the hotplug event to switch sink device on and off periodly.  With DisplayPort 1.2 spec, '' This bit is used for either monitor hotplug/unplug or for notification of a sink event.", I am not sure if it is notification of  a sink event or real hotplug event. We have no code to identify the difference between hotplug/unplug  and notification of a sink event. I check display port spec and also not found how to identify them differently.
>
There are patches on the list to distinguish between short and long
hpd irqs, they are need of review.

Dave.

Wang, Quanxian June 26, 2014, 5:39 a.m. UTC | #18

> -----Original Message-----
> From: Jani Nikula [mailto:[e-mail]]
> Sent: Tuesday, June 24, 2014 11:58 PM
> To: Wang, Quanxian
> Cc: [e-mail]; Daniel Vetter
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable
> for valleyview platform.
> 
> >> Per DP spec we need to read the SINK_COUNT. Perhaps your problem is
> >> the hotplug irqs?
> > [Wang, Quanxian] Hi, Jani
> > The log event is about the transaction event instead of hotplug event. It is
> general since they will use I2c to read byte one by one. It will generate many
> transaction.
> >
> > But the root cause is really about hotplug(intel_hpd_irq_handler). When
> we switch to console, there will be a hotplug event happens after a while.
> After that, the system will continually get the hotplug event to switch sink
> device on and off periodly.  With DisplayPort 1.2 spec, '' This bit is used for
> either monitor hotplug/unplug or for notification of a sink event.", I am not
> sure if it is notification of  a sink event or real hotplug event. We have no
> code to identify the difference between hotplug/unplug  and notification of
> a sink event. I check display port spec and also not found how to identify
> them differently.
> >
> > Question is:
> > In function intel_dp_detect_dpcd, before checking SINK_COUNT, we will
> use intel_dp_get_dpcd to get the dpcd. Could we think there is an active sink
> device attached to branch device if dpcd content is not null.
> > According to the display port spec, only sink device has dpcd, if we get one,
> there must be a sink device attached to branch device or source device. Right?
> 
> No. From your logs, the DPCD has bit 0 set in address 5h, which means
> downstream port present, which is only allowed in branch devices.
[Wang, Quanxian] Ok. Currently I have some founds.  

Sink device attached to branch device will be idle without operation after a while. And kernel will
get the 'fake dpms off' event and called intel_encoder_dpms to disable the connector and dp link will be disabled(See the 1st line of log list below, intel_encoder_dpms will be called with DPMS off). At that time, kernel thought connector had no any sink device attached when SINK_COUNT return 0. Status will be changed from connected to disconnected. Encoder will be deleted after that. Whatever for console terminal or other apps which depends on connector will not work. In this case, The system is in black status even if you press any key or others. From ssh terminal, we could find kernel gets hpd event continually, however at that time, encoder is deleted, connector is not be restored. Always get '[CRTC:6] [NOFB ]'.

So SINK_COUNT is not the only way to check the alive status for connector. If we got one or more, we can confirm that connector is alive. However if we get 0, we should not say connector is not alive. We should try another way to make sure if it is alive. 

I have tried with DDC checking if SINK_COUNT is 0. It works for DP with branch device. But if disconnect the sink device from Branch device, it will not work.
So I *plan*: 
1) when we get SINK_COUNT to be 0, don't return disconnected. We will continue to check DDC.
2) or firstly check OUI to make sure if there is branch device, if there is, then check DDC if SINK_COUNT is 0.

Any suggestion for this process?

[ 1334.170715] [drm:intel_encoder_dpms], mode:3
[ 1334.170721] [drm:intel_crtc_update_dpms],
[ 1334.170726] [drm:i9xx_crtc_disable],
[ 1334.170730] [drm:intel_disable_dp],
[ 1334.170735] [drm:intel_dp_aux_native_write],
[ 1334.170741] [drm:intel_dp_aux_native_write], retry 0
[ 1334.201477] [drm:intel_post_disable_dp],
[ 1334.201483] [drm:intel_dp_link_down],
[ 1334.253549] [drm:g4x_wait_for_vblank], vblank wait timed out
[ 1334.256743] [drm:valleyview_update_wm], Setting FIFO watermarks - A: plane=2, cursor=2, B: plane=2, cursor=2, SR: plane=0, cursor=0
[ 1334.256761] [drm:check_encoder_state], [ENCODER:10:DAC-10]
[ 1334.256769] [drm:check_encoder_state], [ENCODER:11:TMDS-11]
[ 1334.256777] [drm:check_encoder_state], [ENCODER:15:TMDS-15]
[ 1334.256784] [drm:check_crtc_state], [CRTC:3]
[ 1334.256791] [drm:check_crtc_state], [CRTC:6]
[ 1340.097288] [drm:valleyview_irq_handler], hotplug event happens 0x20020000 & 0x007e08c0???
[ 1340.097311] [drm:intel_hpd_irq_handler], Received HPD interrupt on PIN 4 - cnt: 0
[ 1340.097417] [drm:i915_hotplug_work_func], running encoder hotplug functions
[ 1340.097436] [drm:i915_hotplug_work_func], Connector HDMI-A-1 (pin 4) received hotplug event.
[ 1340.097450] [drm:i915_hotplug_work_func], Connector DP-1 (pin 4) received hotplug event.
[ 1340.097464] [drm:intel_hdmi_detect], [CONNECTOR:12:HDMI-A-1]
[ 1340.104895] [drm:drm_do_probe_ddc_edid], drm: skipping non-existent adapter i915 gmbus dpb
[ 1340.104913] [drm:intel_dp_detect], [CONNECTOR:16:DP-1]
[ 1340.104925] [drm:intel_dp_detect_dpcd], Get dpcd
[ 1340.105757] [drm:intel_dp_aux_native_read], aux_ch read and got 15 bytes
[ 1340.105772] [drm:intel_dp_aux_native_read_retry], expected 15, and get 15
[ 1340.105785] [drm:intel_dp_get_dpcd], DPCD: 11 0a 82 01 01 15 01 81 00 01 04 01 0f 00 00
[ 1340.106330] [drm:intel_dp_aux_native_read], aux_ch read and got 16 bytes
[ 1340.106345] [drm:intel_dp_aux_native_read_retry], expected 16, and get 16
[ 1340.106355] [drm:intel_dp_detect_dpcd], Check dpcd
[ 1340.106365] [drm:intel_dp_detect_dpcd], get aux_native reg
[ 1340.106772] [drm:intel_dp_aux_native_read], aux_ch read and got 1 bytes
[ 1340.106786] [drm:intel_dp_aux_native_read_retry], expected 1, and get 1
[ 1340.106798] [drm:intel_dp_detect_dpcd], Check SINK_COUNT reg:0x0, 0
[ 1340.106814] [drm:intel_hpd_irq_event], [CONNECTOR:16:DP-1] status updated from connected to disconnected
[ 1340.108038] [drm:drm_fb_helper_hotplug_event],

Jani Nikula June 30, 2014, 10:45 a.m. UTC | #19

On Thu, 26 Jun 2014, "Wang, Quanxian" <[e-mail]> wrote:
>> -----Original Message-----
>> From: Jani Nikula [mailto:[e-mail]]
>> Sent: Tuesday, June 24, 2014 11:58 PM
>> To: Wang, Quanxian
>> Cc: [e-mail]; Daniel Vetter
>> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable
>> for valleyview platform.
>> 
>> >> Per DP spec we need to read the SINK_COUNT. Perhaps your problem is
>> >> the hotplug irqs?
>> > [Wang, Quanxian] Hi, Jani
>> > The log event is about the transaction event instead of hotplug event. It is
>> general since they will use I2c to read byte one by one. It will generate many
>> transaction.
>> >
>> > But the root cause is really about hotplug(intel_hpd_irq_handler). When
>> we switch to console, there will be a hotplug event happens after a while.
>> After that, the system will continually get the hotplug event to switch sink
>> device on and off periodly.  With DisplayPort 1.2 spec, '' This bit is used for
>> either monitor hotplug/unplug or for notification of a sink event.", I am not
>> sure if it is notification of  a sink event or real hotplug event. We have no
>> code to identify the difference between hotplug/unplug  and notification of
>> a sink event. I check display port spec and also not found how to identify
>> them differently.
>> >
>> > Question is:
>> > In function intel_dp_detect_dpcd, before checking SINK_COUNT, we will
>> use intel_dp_get_dpcd to get the dpcd. Could we think there is an active sink
>> device attached to branch device if dpcd content is not null.
>> > According to the display port spec, only sink device has dpcd, if we get one,
>> there must be a sink device attached to branch device or source device. Right?
>> 
>> No. From your logs, the DPCD has bit 0 set in address 5h, which means
>> downstream port present, which is only allowed in branch devices.
> [Wang, Quanxian] Ok. Currently I have some founds.  
>
> Sink device attached to branch device will be idle without operation after a while. And kernel will
> get the 'fake dpms off' event and called intel_encoder_dpms to disable the connector and dp link will be disabled(See the 1st line of log list below, intel_encoder_dpms will be called with DPMS off). At that time, kernel thought connector had no any sink device attached when SINK_COUNT return 0. Status will be changed from connected to disconnected. Encoder will be deleted after that. Whatever for console terminal or other apps which depends on connector will not work. In this case, The system is in black status even if you press any key or others. From ssh terminal, we could find kernel gets hpd event continually, however at that time, encoder is deleted, connector is not be restored. Always get '[CRTC:6] [NOFB ]'.
>
> So SINK_COUNT is not the only way to check the alive status for connector. If we got one or more, we can confirm that connector is alive. However if we get 0, we should not say connector is not alive. We should try another way to make sure if it is alive. 
>
> I have tried with DDC checking if SINK_COUNT is 0. It works for DP
> with branch device. But if disconnect the sink device from Branch
> device, it will not work.

Huh, if you disconnect the sink device from branch device, SINK_COUNT is
expected to be 0, and the result is expected to be disconnected, right?!

BR,
Jani.


> So I *plan*: 
> 1) when we get SINK_COUNT to be 0, don't return disconnected. We will continue to check DDC.
> 2) or firstly check OUI to make sure if there is branch device, if there is, then check DDC if SINK_COUNT is 0.
>
> Any suggestion for this process?
>
> [ 1334.170715] [drm:intel_encoder_dpms], mode:3
> [ 1334.170721] [drm:intel_crtc_update_dpms],
> [ 1334.170726] [drm:i9xx_crtc_disable],
> [ 1334.170730] [drm:intel_disable_dp],
> [ 1334.170735] [drm:intel_dp_aux_native_write],
> [ 1334.170741] [drm:intel_dp_aux_native_write], retry 0
> [ 1334.201477] [drm:intel_post_disable_dp],
> [ 1334.201483] [drm:intel_dp_link_down],
> [ 1334.253549] [drm:g4x_wait_for_vblank], vblank wait timed out
> [ 1334.256743] [drm:valleyview_update_wm], Setting FIFO watermarks - A: plane=2, cursor=2, B: plane=2, cursor=2, SR: plane=0, cursor=0
> [ 1334.256761] [drm:check_encoder_state], [ENCODER:10:DAC-10]
> [ 1334.256769] [drm:check_encoder_state], [ENCODER:11:TMDS-11]
> [ 1334.256777] [drm:check_encoder_state], [ENCODER:15:TMDS-15]
> [ 1334.256784] [drm:check_crtc_state], [CRTC:3]
> [ 1334.256791] [drm:check_crtc_state], [CRTC:6]
> [ 1340.097288] [drm:valleyview_irq_handler], hotplug event happens 0x20020000 & 0x007e08c0???
> [ 1340.097311] [drm:intel_hpd_irq_handler], Received HPD interrupt on PIN 4 - cnt: 0
> [ 1340.097417] [drm:i915_hotplug_work_func], running encoder hotplug functions
> [ 1340.097436] [drm:i915_hotplug_work_func], Connector HDMI-A-1 (pin 4) received hotplug event.
> [ 1340.097450] [drm:i915_hotplug_work_func], Connector DP-1 (pin 4) received hotplug event.
> [ 1340.097464] [drm:intel_hdmi_detect], [CONNECTOR:12:HDMI-A-1]
> [ 1340.104895] [drm:drm_do_probe_ddc_edid], drm: skipping non-existent adapter i915 gmbus dpb
> [ 1340.104913] [drm:intel_dp_detect], [CONNECTOR:16:DP-1]
> [ 1340.104925] [drm:intel_dp_detect_dpcd], Get dpcd
> [ 1340.105757] [drm:intel_dp_aux_native_read], aux_ch read and got 15 bytes
> [ 1340.105772] [drm:intel_dp_aux_native_read_retry], expected 15, and get 15
> [ 1340.105785] [drm:intel_dp_get_dpcd], DPCD: 11 0a 82 01 01 15 01 81 00 01 04 01 0f 00 00
> [ 1340.106330] [drm:intel_dp_aux_native_read], aux_ch read and got 16 bytes
> [ 1340.106345] [drm:intel_dp_aux_native_read_retry], expected 16, and get 16
> [ 1340.106355] [drm:intel_dp_detect_dpcd], Check dpcd
> [ 1340.106365] [drm:intel_dp_detect_dpcd], get aux_native reg
> [ 1340.106772] [drm:intel_dp_aux_native_read], aux_ch read and got 1 bytes
> [ 1340.106786] [drm:intel_dp_aux_native_read_retry], expected 1, and get 1
> [ 1340.106798] [drm:intel_dp_detect_dpcd], Check SINK_COUNT reg:0x0, 0
> [ 1340.106814] [drm:intel_hpd_irq_event], [CONNECTOR:16:DP-1] status updated from connected to disconnected
> [ 1340.108038] [drm:drm_fb_helper_hotplug_event],
>
>
>

Wang, Quanxian July 1, 2014, 2:03 a.m. UTC | #20

> -----Original Message-----
> From: Jani Nikula [mailto:[e-mail]]
> Sent: Monday, June 30, 2014 6:45 PM
> To: Wang, Quanxian
> Cc: [e-mail]; Daniel Vetter
> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not reliable
> for valleyview platform.
> 
> On Thu, 26 Jun 2014, "Wang, Quanxian" <[e-mail]> wrote:
> >> -----Original Message-----
> >> From: Jani Nikula [mailto:[e-mail]]
> >> Sent: Tuesday, June 24, 2014 11:58 PM
> >> To: Wang, Quanxian
> >> Cc: [e-mail]; Daniel Vetter
> >> Subject: RE: [Intel-gfx] [PATCH] drm/i915/vlv: DP_SINK_COUNT is not
> >> reliable for valleyview platform.
> >>
> >> >> Per DP spec we need to read the SINK_COUNT. Perhaps your problem
> >> >> is the hotplug irqs?
> >> > [Wang, Quanxian] Hi, Jani
> >> > The log event is about the transaction event instead of hotplug
> >> > event. It is
> >> general since they will use I2c to read byte one by one. It will
> >> generate many transaction.
> >> >
> >> > But the root cause is really about hotplug(intel_hpd_irq_handler).
> >> > When
> >> we switch to console, there will be a hotplug event happens after a while.
> >> After that, the system will continually get the hotplug event to
> >> switch sink device on and off periodly.  With DisplayPort 1.2 spec,
> >> '' This bit is used for either monitor hotplug/unplug or for
> >> notification of a sink event.", I am not sure if it is notification
> >> of  a sink event or real hotplug event. We have no code to identify
> >> the difference between hotplug/unplug  and notification of a sink
> >> event. I check display port spec and also not found how to identify them
> differently.
> >> >
> >> > Question is:
> >> > In function intel_dp_detect_dpcd, before checking SINK_COUNT, we
> >> > will
> >> use intel_dp_get_dpcd to get the dpcd. Could we think there is an
> >> active sink device attached to branch device if dpcd content is not null.
> >> > According to the display port spec, only sink device has dpcd, if
> >> > we get one,
> >> there must be a sink device attached to branch device or source device.
> Right?
> >>
> >> No. From your logs, the DPCD has bit 0 set in address 5h, which means
> >> downstream port present, which is only allowed in branch devices.
> > [Wang, Quanxian] Ok. Currently I have some founds.
> >
> > Sink device attached to branch device will be idle without operation
> > after a while. And kernel will get the 'fake dpms off' event and called
> intel_encoder_dpms to disable the connector and dp link will be
> disabled(See the 1st line of log list below, intel_encoder_dpms will be called
> with DPMS off). At that time, kernel thought connector had no any sink
> device attached when SINK_COUNT return 0. Status will be changed from
> connected to disconnected. Encoder will be deleted after that. Whatever for
> console terminal or other apps which depends on connector will not work. In
> this case, The system is in black status even if you press any key or others.
> From ssh terminal, we could find kernel gets hpd event continually, however
> at that time, encoder is deleted, connector is not be restored. Always get
> '[CRTC:6] [NOFB ]'.
> >
> > So SINK_COUNT is not the only way to check the alive status for connector.
> If we got one or more, we can confirm that connector is alive. However if we
> get 0, we should not say connector is not alive. We should try another way to
> make sure if it is alive.
> >
> > I have tried with DDC checking if SINK_COUNT is 0. It works for DP
> > with branch device. But if disconnect the sink device from Branch
> > device, it will not work.
> 
> Huh, if you disconnect the sink device from branch device, SINK_COUNT is
> expected to be 0, and the result is expected to be disconnected, right?!
[Wang, Quanxian] No, I don't disconnect the sink device physically. Kernel will receive the dpms signal when branch device think monitor is in sleep status(The screen turns to black after a while). But it is still connected. Kernel still could get the DDC data by calling drm_probe_ddc and also kernel still could get EDID data. From DP protocol, SINK_COUNT is 0 and it doesn't show the real status of branch device at that time.
That is why I think SINK_COUNT isn't reliable. With this patch, when I press any key, monitor will turn to be active from sleep status. Without it, any action on key or mouse could not active monitor after its sleep.
> 
> BR,
> Jani.
> 
> 
> > So I *plan*:
> > 1) when we get SINK_COUNT to be 0, don't return disconnected. We will
> continue to check DDC.
> > 2) or firstly check OUI to make sure if there is branch device, if there is,
> then check DDC if SINK_COUNT is 0.
> >
> > Any suggestion for this process?
> >
> > [ 1334.170715] [drm:intel_encoder_dpms], mode:3 [ 1334.170721]
> > [drm:intel_crtc_update_dpms], [ 1334.170726] [drm:i9xx_crtc_disable],
> > [ 1334.170730] [drm:intel_disable_dp], [ 1334.170735]
> > [drm:intel_dp_aux_native_write], [ 1334.170741]
> > [drm:intel_dp_aux_native_write], retry 0 [ 1334.201477]
> > [drm:intel_post_disable_dp], [ 1334.201483] [drm:intel_dp_link_down],
> > [ 1334.253549] [drm:g4x_wait_for_vblank], vblank wait timed out [
> > 1334.256743] [drm:valleyview_update_wm], Setting FIFO watermarks - A:
> > plane=2, cursor=2, B: plane=2, cursor=2, SR: plane=0, cursor=0 [
> > 1334.256761] [drm:check_encoder_state], [ENCODER:10:DAC-10] [
> > 1334.256769] [drm:check_encoder_state], [ENCODER:11:TMDS-11] [
> > 1334.256777] [drm:check_encoder_state], [ENCODER:15:TMDS-15] [
> > 1334.256784] [drm:check_crtc_state], [CRTC:3] [ 1334.256791]
> > [drm:check_crtc_state], [CRTC:6] [ 1340.097288]
> [drm:valleyview_irq_handler], hotplug event happens 0x20020000 &
> 0x007e08c0???
> > [ 1340.097311] [drm:intel_hpd_irq_handler], Received HPD interrupt on
> > PIN 4 - cnt: 0 [ 1340.097417] [drm:i915_hotplug_work_func], running
> > encoder hotplug functions [ 1340.097436] [drm:i915_hotplug_work_func],
> Connector HDMI-A-1 (pin 4) received hotplug event.
> > [ 1340.097450] [drm:i915_hotplug_work_func], Connector DP-1 (pin 4)
> received hotplug event.
> > [ 1340.097464] [drm:intel_hdmi_detect], [CONNECTOR:12:HDMI-A-1] [
> > 1340.104895] [drm:drm_do_probe_ddc_edid], drm: skipping non-existent
> > adapter i915 gmbus dpb [ 1340.104913] [drm:intel_dp_detect],
> > [CONNECTOR:16:DP-1] [ 1340.104925] [drm:intel_dp_detect_dpcd], Get
> > dpcd [ 1340.105757] [drm:intel_dp_aux_native_read], aux_ch read and
> > got 15 bytes [ 1340.105772] [drm:intel_dp_aux_native_read_retry],
> > expected 15, and get 15 [ 1340.105785] [drm:intel_dp_get_dpcd], DPCD:
> > 11 0a 82 01 01 15 01 81 00 01 04 01 0f 00 00 [ 1340.106330]
> > [drm:intel_dp_aux_native_read], aux_ch read and got 16 bytes [
> > 1340.106345] [drm:intel_dp_aux_native_read_retry], expected 16, and
> > get 16 [ 1340.106355] [drm:intel_dp_detect_dpcd], Check dpcd [
> > 1340.106365] [drm:intel_dp_detect_dpcd], get aux_native reg [
> > 1340.106772] [drm:intel_dp_aux_native_read], aux_ch read and got 1
> > bytes [ 1340.106786] [drm:intel_dp_aux_native_read_retry], expected 1,
> > and get 1 [ 1340.106798] [drm:intel_dp_detect_dpcd], Check SINK_COUNT
> > reg:0x0, 0 [ 1340.106814] [drm:intel_hpd_irq_event],
> > [CONNECTOR:16:DP-1] status updated from connected to disconnected [
> > 1340.108038] [drm:drm_fb_helper_hotplug_event],
> >
> >
> >
> 
> --
> Jani Nikula, Intel Open Source Technology Center

Patch

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,