* [PATCH] drm/i915: Improve DP downstream HPD handling
@ 2015-07-06 12:12 ville.syrjala
2015-07-07 9:56 ` Sivakumar Thulasimani
2015-07-07 11:23 ` shuang.he
0 siblings, 2 replies; 20+ messages in thread
From: ville.syrjala @ 2015-07-06 12:12 UTC (permalink / raw)
To: intel-gfx
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
DP dongles may signal downstream HPD via short HPD pulses. If we know
the device has a HPD capable downstream port, make sure we kick off the
full hotplug processing even for short HPDs.
Additonally setting the sink to DPMS off kills the downstream HPD (at
least on my DP->VGA dongle), so skip the DPMS off for such dongles
when we turn off the port.
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e88cec2..f424833 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2324,6 +2324,13 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
}
}
+static bool intel_dp_has_downstream_hpd(struct intel_dp *intel_dp)
+{
+ return intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
+ intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
+ intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
+}
+
static void intel_disable_dp(struct intel_encoder *encoder)
{
struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
@@ -2340,7 +2347,9 @@ static void intel_disable_dp(struct intel_encoder *encoder)
* ensure that we have vdd while we switch off the panel. */
intel_edp_panel_vdd_on(intel_dp);
intel_edp_backlight_off(intel_dp);
- intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
+ /* Skip power down to keep downstream HPD working */
+ if (!intel_dp_has_downstream_hpd(intel_dp))
+ intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
intel_edp_panel_off(intel_dp);
/* disable the port before the pipe on g4x */
@@ -4944,6 +4953,13 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
intel_dp_check_link_status(intel_dp);
drm_modeset_unlock(&dev->mode_config.connection_mutex);
+
+ /*
+ * Downstream HPD will generate a short HPD,
+ * so we want full hotplug processing here.
+ */
+ if (intel_dp_has_downstream_hpd(intel_dp))
+ goto put_power;
}
}
--
2.3.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Improve DP downstream HPD handling
2015-07-06 12:12 [PATCH] drm/i915: Improve DP downstream HPD handling ville.syrjala
@ 2015-07-07 9:56 ` Sivakumar Thulasimani
2015-07-07 11:10 ` Ville Syrjälä
2015-07-07 11:23 ` shuang.he
1 sibling, 1 reply; 20+ messages in thread
From: Sivakumar Thulasimani @ 2015-07-07 9:56 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx
On 7/6/2015 5:42 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> DP dongles may signal downstream HPD via short HPD pulses. If we know
> the device has a HPD capable downstream port, make sure we kick off the
> full hotplug processing even for short HPDs.
>
> Additonally setting the sink to DPMS off kills the downstream HPD (at
> least on my DP->VGA dongle), so skip the DPMS off for such dongles
> when we turn off the port.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e88cec2..f424833 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2324,6 +2324,13 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> }
> }
>
> +static bool intel_dp_has_downstream_hpd(struct intel_dp *intel_dp)
> +{
> + return intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
> + intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
> +}
> +
> static void intel_disable_dp(struct intel_encoder *encoder)
> {
> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> @@ -2340,7 +2347,9 @@ static void intel_disable_dp(struct intel_encoder *encoder)
> * ensure that we have vdd while we switch off the panel. */
> intel_edp_panel_vdd_on(intel_dp);
> intel_edp_backlight_off(intel_dp);
> - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> + /* Skip power down to keep downstream HPD working */
> + if (!intel_dp_has_downstream_hpd(intel_dp))
> + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> intel_edp_panel_off(intel_dp);
>
> /* disable the port before the pipe on g4x */
> @@ -4944,6 +4953,13 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> intel_dp_check_link_status(intel_dp);
> drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +
> + /*
> + * Downstream HPD will generate a short HPD,
> + * so we want full hotplug processing here.
> + */
> + if (intel_dp_has_downstream_hpd(intel_dp))
> + goto put_power;
> }
> }
>
I am looking into compliance changes for DP and this seems a relevant
change for compliance as well. but as per Link CTS 1.2 section 4.2.2.8,
we are supposed to read the sink_count and do full detection if
sink_count is >1. So instead of checking for DP_DS_PORT_HPD can we just
check SINK_COUNT and do full detect ?
--
regards,
Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Improve DP downstream HPD handling
2015-07-07 9:56 ` Sivakumar Thulasimani
@ 2015-07-07 11:10 ` Ville Syrjälä
2015-07-07 11:15 ` Sivakumar Thulasimani
0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2015-07-07 11:10 UTC (permalink / raw)
To: Sivakumar Thulasimani; +Cc: intel-gfx
On Tue, Jul 07, 2015 at 03:26:36PM +0530, Sivakumar Thulasimani wrote:
>
>
> On 7/6/2015 5:42 PM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > DP dongles may signal downstream HPD via short HPD pulses. If we know
> > the device has a HPD capable downstream port, make sure we kick off the
> > full hotplug processing even for short HPDs.
> >
> > Additonally setting the sink to DPMS off kills the downstream HPD (at
> > least on my DP->VGA dongle), so skip the DPMS off for such dongles
> > when we turn off the port.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index e88cec2..f424833 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2324,6 +2324,13 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> > }
> > }
> >
> > +static bool intel_dp_has_downstream_hpd(struct intel_dp *intel_dp)
> > +{
> > + return intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
> > + intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> > + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
> > +}
> > +
> > static void intel_disable_dp(struct intel_encoder *encoder)
> > {
> > struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > @@ -2340,7 +2347,9 @@ static void intel_disable_dp(struct intel_encoder *encoder)
> > * ensure that we have vdd while we switch off the panel. */
> > intel_edp_panel_vdd_on(intel_dp);
> > intel_edp_backlight_off(intel_dp);
> > - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> > + /* Skip power down to keep downstream HPD working */
> > + if (!intel_dp_has_downstream_hpd(intel_dp))
> > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> > intel_edp_panel_off(intel_dp);
> >
> > /* disable the port before the pipe on g4x */
> > @@ -4944,6 +4953,13 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> > drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> > intel_dp_check_link_status(intel_dp);
> > drm_modeset_unlock(&dev->mode_config.connection_mutex);
> > +
> > + /*
> > + * Downstream HPD will generate a short HPD,
> > + * so we want full hotplug processing here.
> > + */
> > + if (intel_dp_has_downstream_hpd(intel_dp))
> > + goto put_power;
> > }
> > }
> >
> I am looking into compliance changes for DP and this seems a relevant
> change for compliance as well. but as per Link CTS 1.2 section 4.2.2.8,
> we are supposed to read the sink_count and do full detection if
> sink_count is >1. So instead of checking for DP_DS_PORT_HPD can we just
> check SINK_COUNT and do full detect ?
->detect() will be called from the hotplug work and that will
check SINK_COUNT.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Improve DP downstream HPD handling
2015-07-07 11:10 ` Ville Syrjälä
@ 2015-07-07 11:15 ` Sivakumar Thulasimani
2015-07-07 11:37 ` Ville Syrjälä
0 siblings, 1 reply; 20+ messages in thread
From: Sivakumar Thulasimani @ 2015-07-07 11:15 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On 7/7/2015 4:40 PM, Ville Syrjälä wrote:
> On Tue, Jul 07, 2015 at 03:26:36PM +0530, Sivakumar Thulasimani wrote:
>>
>> On 7/6/2015 5:42 PM, ville.syrjala@linux.intel.com wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> DP dongles may signal downstream HPD via short HPD pulses. If we know
>>> the device has a HPD capable downstream port, make sure we kick off the
>>> full hotplug processing even for short HPDs.
>>>
>>> Additonally setting the sink to DPMS off kills the downstream HPD (at
>>> least on my DP->VGA dongle), so skip the DPMS off for such dongles
>>> when we turn off the port.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>> drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
>>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index e88cec2..f424833 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -2324,6 +2324,13 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>>> }
>>> }
>>>
>>> +static bool intel_dp_has_downstream_hpd(struct intel_dp *intel_dp)
>>> +{
>>> + return intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
>>> + intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
>>> + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
>>> +}
>>> +
>>> static void intel_disable_dp(struct intel_encoder *encoder)
>>> {
>>> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>>> @@ -2340,7 +2347,9 @@ static void intel_disable_dp(struct intel_encoder *encoder)
>>> * ensure that we have vdd while we switch off the panel. */
>>> intel_edp_panel_vdd_on(intel_dp);
>>> intel_edp_backlight_off(intel_dp);
>>> - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>>> + /* Skip power down to keep downstream HPD working */
>>> + if (!intel_dp_has_downstream_hpd(intel_dp))
>>> + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>>> intel_edp_panel_off(intel_dp);
>>>
>>> /* disable the port before the pipe on g4x */
>>> @@ -4944,6 +4953,13 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>>> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>>> intel_dp_check_link_status(intel_dp);
>>> drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>> +
>>> + /*
>>> + * Downstream HPD will generate a short HPD,
>>> + * so we want full hotplug processing here.
>>> + */
>>> + if (intel_dp_has_downstream_hpd(intel_dp))
>>> + goto put_power;
>>> }
>>> }
>>>
>> I am looking into compliance changes for DP and this seems a relevant
>> change for compliance as well. but as per Link CTS 1.2 section 4.2.2.8,
>> we are supposed to read the sink_count and do full detection if
>> sink_count is >1. So instead of checking for DP_DS_PORT_HPD can we just
>> check SINK_COUNT and do full detect ?
> ->detect() will be called from the hotplug work and that will
> check SINK_COUNT.
>
No, the Compliance Sink tool, will not set the DP_DS_PORT_HPD resulting
in detect not getting executed for
the short pulse generated. The spec requires the sink to set only the
sink count so it is not a must for
the sink to update the DP_DOWNSTREAM_PORT_0. so only a check for
SINK_COUNT will pass the
compliance test.
--
regards,
Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Improve DP downstream HPD handling
2015-07-06 12:12 [PATCH] drm/i915: Improve DP downstream HPD handling ville.syrjala
2015-07-07 9:56 ` Sivakumar Thulasimani
@ 2015-07-07 11:23 ` shuang.he
1 sibling, 0 replies; 20+ messages in thread
From: shuang.he @ 2015-07-07 11:23 UTC (permalink / raw)
To: shuang.he, lei.a.liu, intel-gfx, ville.syrjala
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6728
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
ILK -1 302/302 301/302
SNB 312/316 312/316
IVB 345/345 345/345
BYT -1 289/289 288/289
HSW 382/382 382/382
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*ILK igt@kms_flip@flip-vs-rmfb-interruptible PASS(1) DMESG_WARN(1)
(dmesg patch applied)drm:intel_pch_fifo_underrun_irq_handler[i915]]*ERROR*PCH_transcoder_A_FIFO_underrun@PCH transcoder A FIFO underrun
*BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Improve DP downstream HPD handling
2015-07-07 11:15 ` Sivakumar Thulasimani
@ 2015-07-07 11:37 ` Ville Syrjälä
2015-07-07 11:54 ` Ville Syrjälä
0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2015-07-07 11:37 UTC (permalink / raw)
To: Sivakumar Thulasimani; +Cc: intel-gfx
On Tue, Jul 07, 2015 at 04:45:11PM +0530, Sivakumar Thulasimani wrote:
>
>
> On 7/7/2015 4:40 PM, Ville Syrjälä wrote:
> > On Tue, Jul 07, 2015 at 03:26:36PM +0530, Sivakumar Thulasimani wrote:
> >>
> >> On 7/6/2015 5:42 PM, ville.syrjala@linux.intel.com wrote:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> DP dongles may signal downstream HPD via short HPD pulses. If we know
> >>> the device has a HPD capable downstream port, make sure we kick off the
> >>> full hotplug processing even for short HPDs.
> >>>
> >>> Additonally setting the sink to DPMS off kills the downstream HPD (at
> >>> least on my DP->VGA dongle), so skip the DPMS off for such dongles
> >>> when we turn off the port.
> >>>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>> drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
> >>> 1 file changed, 17 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >>> index e88cec2..f424833 100644
> >>> --- a/drivers/gpu/drm/i915/intel_dp.c
> >>> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >>> @@ -2324,6 +2324,13 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> >>> }
> >>> }
> >>>
> >>> +static bool intel_dp_has_downstream_hpd(struct intel_dp *intel_dp)
> >>> +{
> >>> + return intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
> >>> + intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> >>> + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
> >>> +}
> >>> +
> >>> static void intel_disable_dp(struct intel_encoder *encoder)
> >>> {
> >>> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> >>> @@ -2340,7 +2347,9 @@ static void intel_disable_dp(struct intel_encoder *encoder)
> >>> * ensure that we have vdd while we switch off the panel. */
> >>> intel_edp_panel_vdd_on(intel_dp);
> >>> intel_edp_backlight_off(intel_dp);
> >>> - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> >>> + /* Skip power down to keep downstream HPD working */
> >>> + if (!intel_dp_has_downstream_hpd(intel_dp))
> >>> + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> >>> intel_edp_panel_off(intel_dp);
> >>>
> >>> /* disable the port before the pipe on g4x */
> >>> @@ -4944,6 +4953,13 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >>> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> >>> intel_dp_check_link_status(intel_dp);
> >>> drm_modeset_unlock(&dev->mode_config.connection_mutex);
> >>> +
> >>> + /*
> >>> + * Downstream HPD will generate a short HPD,
> >>> + * so we want full hotplug processing here.
> >>> + */
> >>> + if (intel_dp_has_downstream_hpd(intel_dp))
> >>> + goto put_power;
> >>> }
> >>> }
> >>>
> >> I am looking into compliance changes for DP and this seems a relevant
> >> change for compliance as well. but as per Link CTS 1.2 section 4.2.2.8,
> >> we are supposed to read the sink_count and do full detection if
> >> sink_count is >1. So instead of checking for DP_DS_PORT_HPD can we just
> >> check SINK_COUNT and do full detect ?
> > ->detect() will be called from the hotplug work and that will
> > check SINK_COUNT.
> >
> No, the Compliance Sink tool, will not set the DP_DS_PORT_HPD resulting
> in detect not getting executed for
> the short pulse generated. The spec requires the sink to set only the
> sink count so it is not a must for
> the sink to update the DP_DOWNSTREAM_PORT_0. so only a check for
> SINK_COUNT will pass the
> compliance test.
That seems stupid. If the downstream port isn't HPD capable then we have
no reason to check SINK_COUNT after a short HPD as the short HPD
coudln't have been caused by a downstram HPD. Obviuously we still
check SINK_COUNT after a long HPD to figure out if anything is connected
when the branch device itself gets connected to the source.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Improve DP downstream HPD handling
2015-07-07 11:37 ` Ville Syrjälä
@ 2015-07-07 11:54 ` Ville Syrjälä
2015-07-07 12:20 ` Sivakumar Thulasimani
0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2015-07-07 11:54 UTC (permalink / raw)
To: Sivakumar Thulasimani; +Cc: intel-gfx
On Tue, Jul 07, 2015 at 02:37:46PM +0300, Ville Syrjälä wrote:
> On Tue, Jul 07, 2015 at 04:45:11PM +0530, Sivakumar Thulasimani wrote:
> >
> >
> > On 7/7/2015 4:40 PM, Ville Syrjälä wrote:
> > > On Tue, Jul 07, 2015 at 03:26:36PM +0530, Sivakumar Thulasimani wrote:
> > >>
> > >> On 7/6/2015 5:42 PM, ville.syrjala@linux.intel.com wrote:
> > >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >>>
> > >>> DP dongles may signal downstream HPD via short HPD pulses. If we know
> > >>> the device has a HPD capable downstream port, make sure we kick off the
> > >>> full hotplug processing even for short HPDs.
> > >>>
> > >>> Additonally setting the sink to DPMS off kills the downstream HPD (at
> > >>> least on my DP->VGA dongle), so skip the DPMS off for such dongles
> > >>> when we turn off the port.
> > >>>
> > >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >>> ---
> > >>> drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
> > >>> 1 file changed, 17 insertions(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > >>> index e88cec2..f424833 100644
> > >>> --- a/drivers/gpu/drm/i915/intel_dp.c
> > >>> +++ b/drivers/gpu/drm/i915/intel_dp.c
> > >>> @@ -2324,6 +2324,13 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> > >>> }
> > >>> }
> > >>>
> > >>> +static bool intel_dp_has_downstream_hpd(struct intel_dp *intel_dp)
> > >>> +{
> > >>> + return intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
> > >>> + intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> > >>> + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
> > >>> +}
> > >>> +
> > >>> static void intel_disable_dp(struct intel_encoder *encoder)
> > >>> {
> > >>> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > >>> @@ -2340,7 +2347,9 @@ static void intel_disable_dp(struct intel_encoder *encoder)
> > >>> * ensure that we have vdd while we switch off the panel. */
> > >>> intel_edp_panel_vdd_on(intel_dp);
> > >>> intel_edp_backlight_off(intel_dp);
> > >>> - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> > >>> + /* Skip power down to keep downstream HPD working */
> > >>> + if (!intel_dp_has_downstream_hpd(intel_dp))
> > >>> + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> > >>> intel_edp_panel_off(intel_dp);
> > >>>
> > >>> /* disable the port before the pipe on g4x */
> > >>> @@ -4944,6 +4953,13 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> > >>> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> > >>> intel_dp_check_link_status(intel_dp);
> > >>> drm_modeset_unlock(&dev->mode_config.connection_mutex);
> > >>> +
> > >>> + /*
> > >>> + * Downstream HPD will generate a short HPD,
> > >>> + * so we want full hotplug processing here.
> > >>> + */
> > >>> + if (intel_dp_has_downstream_hpd(intel_dp))
> > >>> + goto put_power;
> > >>> }
> > >>> }
> > >>>
> > >> I am looking into compliance changes for DP and this seems a relevant
> > >> change for compliance as well. but as per Link CTS 1.2 section 4.2.2.8,
> > >> we are supposed to read the sink_count and do full detection if
> > >> sink_count is >1. So instead of checking for DP_DS_PORT_HPD can we just
> > >> check SINK_COUNT and do full detect ?
> > > ->detect() will be called from the hotplug work and that will
> > > check SINK_COUNT.
> > >
> > No, the Compliance Sink tool, will not set the DP_DS_PORT_HPD resulting
> > in detect not getting executed for
> > the short pulse generated. The spec requires the sink to set only the
> > sink count so it is not a must for
> > the sink to update the DP_DOWNSTREAM_PORT_0. so only a check for
> > SINK_COUNT will pass the
> > compliance test.
>
> That seems stupid. If the downstream port isn't HPD capable then we have
> no reason to check SINK_COUNT after a short HPD as the short HPD
> coudln't have been caused by a downstram HPD. Obviuously we still
> check SINK_COUNT after a long HPD to figure out if anything is connected
> when the branch device itself gets connected to the source.
Actually that's not correct. We don't check SINK_COUNT unless the downstream
port is HPD capable.
The spec says:
"If the DFP does not provide for means for plug/unplug detection, the
adaptor must set the SINK_COUNT field bits, as if those Sink devices were
all permanently plugged."
So according to the there can't be any changes in SINK_COUNT if the
downstream port is not HPD capable.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Improve DP downstream HPD handling
2015-07-07 11:54 ` Ville Syrjälä
@ 2015-07-07 12:20 ` Sivakumar Thulasimani
2015-07-08 12:20 ` Sivakumar Thulasimani
0 siblings, 1 reply; 20+ messages in thread
From: Sivakumar Thulasimani @ 2015-07-07 12:20 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On 7/7/2015 5:24 PM, Ville Syrjälä wrote:
> On Tue, Jul 07, 2015 at 02:37:46PM +0300, Ville Syrjälä wrote:
>> On Tue, Jul 07, 2015 at 04:45:11PM +0530, Sivakumar Thulasimani wrote:
>>>
>>> On 7/7/2015 4:40 PM, Ville Syrjälä wrote:
>>>> On Tue, Jul 07, 2015 at 03:26:36PM +0530, Sivakumar Thulasimani wrote:
>>>>> On 7/6/2015 5:42 PM, ville.syrjala@linux.intel.com wrote:
>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>
>>>>>> DP dongles may signal downstream HPD via short HPD pulses. If we know
>>>>>> the device has a HPD capable downstream port, make sure we kick off the
>>>>>> full hotplug processing even for short HPDs.
>>>>>>
>>>>>> Additonally setting the sink to DPMS off kills the downstream HPD (at
>>>>>> least on my DP->VGA dongle), so skip the DPMS off for such dongles
>>>>>> when we turn off the port.
>>>>>>
>>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>> ---
>>>>>> drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
>>>>>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>>>>> index e88cec2..f424833 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>>>> @@ -2324,6 +2324,13 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> +static bool intel_dp_has_downstream_hpd(struct intel_dp *intel_dp)
>>>>>> +{
>>>>>> + return intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
>>>>>> + intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
>>>>>> + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
>>>>>> +}
>>>>>> +
>>>>>> static void intel_disable_dp(struct intel_encoder *encoder)
>>>>>> {
>>>>>> struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>>>>>> @@ -2340,7 +2347,9 @@ static void intel_disable_dp(struct intel_encoder *encoder)
>>>>>> * ensure that we have vdd while we switch off the panel. */
>>>>>> intel_edp_panel_vdd_on(intel_dp);
>>>>>> intel_edp_backlight_off(intel_dp);
>>>>>> - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>>>>>> + /* Skip power down to keep downstream HPD working */
>>>>>> + if (!intel_dp_has_downstream_hpd(intel_dp))
>>>>>> + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>>>>>> intel_edp_panel_off(intel_dp);
>>>>>>
>>>>>> /* disable the port before the pipe on g4x */
>>>>>> @@ -4944,6 +4953,13 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>>>>>> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>>>>>> intel_dp_check_link_status(intel_dp);
>>>>>> drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>>>>> +
>>>>>> + /*
>>>>>> + * Downstream HPD will generate a short HPD,
>>>>>> + * so we want full hotplug processing here.
>>>>>> + */
>>>>>> + if (intel_dp_has_downstream_hpd(intel_dp))
>>>>>> + goto put_power;
>>>>>> }
>>>>>> }
>>>>>>
>>>>> I am looking into compliance changes for DP and this seems a relevant
>>>>> change for compliance as well. but as per Link CTS 1.2 section 4.2.2.8,
>>>>> we are supposed to read the sink_count and do full detection if
>>>>> sink_count is >1. So instead of checking for DP_DS_PORT_HPD can we just
>>>>> check SINK_COUNT and do full detect ?
>>>> ->detect() will be called from the hotplug work and that will
>>>> check SINK_COUNT.
>>>>
>>> No, the Compliance Sink tool, will not set the DP_DS_PORT_HPD resulting
>>> in detect not getting executed for
>>> the short pulse generated. The spec requires the sink to set only the
>>> sink count so it is not a must for
>>> the sink to update the DP_DOWNSTREAM_PORT_0. so only a check for
>>> SINK_COUNT will pass the
>>> compliance test.
>> That seems stupid. If the downstream port isn't HPD capable then we have
>> no reason to check SINK_COUNT after a short HPD as the short HPD
>> coudln't have been caused by a downstram HPD. Obviuously we still
>> check SINK_COUNT after a long HPD to figure out if anything is connected
>> when the branch device itself gets connected to the source.
> Actually that's not correct. We don't check SINK_COUNT unless the downstream
> port is HPD capable.
>
> The spec says:
> "If the DFP does not provide for means for plug/unplug detection, the
> adaptor must set the SINK_COUNT field bits, as if those Sink devices were
> all permanently plugged."
>
> So according to the there can't be any changes in SINK_COUNT if the
> downstream port is not HPD capable.
>
>
>
yes, agree on the no changes for SINK_COUNT if HPD is 0. i'll check with
DP Compliance test
tomorrow and confirm the exact reason for its failure may be my
understanding of it was incorrect.
--
regards,
Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Improve DP downstream HPD handling
2015-07-07 12:20 ` Sivakumar Thulasimani
@ 2015-07-08 12:20 ` Sivakumar Thulasimani
2015-07-08 12:27 ` Ville Syrjälä
0 siblings, 1 reply; 20+ messages in thread
From: Sivakumar Thulasimani @ 2015-07-08 12:20 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
On 7/7/2015 5:50 PM, Sivakumar Thulasimani wrote:
>
>
> On 7/7/2015 5:24 PM, Ville Syrjälä wrote:
>> On Tue, Jul 07, 2015 at 02:37:46PM +0300, Ville Syrjälä wrote:
>>> On Tue, Jul 07, 2015 at 04:45:11PM +0530, Sivakumar Thulasimani wrote:
>>>>
>>>> On 7/7/2015 4:40 PM, Ville Syrjälä wrote:
>>>>> On Tue, Jul 07, 2015 at 03:26:36PM +0530, Sivakumar Thulasimani
>>>>> wrote:
>>>>>> On 7/6/2015 5:42 PM, ville.syrjala@linux.intel.com wrote:
>>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>>
>>>>>>> DP dongles may signal downstream HPD via short HPD pulses. If we
>>>>>>> know
>>>>>>> the device has a HPD capable downstream port, make sure we kick
>>>>>>> off the
>>>>>>> full hotplug processing even for short HPDs.
>>>>>>>
>>>>>>> Additonally setting the sink to DPMS off kills the downstream
>>>>>>> HPD (at
>>>>>>> least on my DP->VGA dongle), so skip the DPMS off for such dongles
>>>>>>> when we turn off the port.
>>>>>>>
>>>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>> ---
>>>>>>> drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
>>>>>>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>>>>>>> b/drivers/gpu/drm/i915/intel_dp.c
>>>>>>> index e88cec2..f424833 100644
>>>>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>>>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>>>>>> @@ -2324,6 +2324,13 @@ static void intel_dp_get_config(struct
>>>>>>> intel_encoder *encoder,
>>>>>>> }
>>>>>>> }
>>>>>>> +static bool intel_dp_has_downstream_hpd(struct intel_dp
>>>>>>> *intel_dp)
>>>>>>> +{
>>>>>>> + return intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
>>>>>>> DP_DWN_STRM_PORT_PRESENT &&
>>>>>>> + intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
>>>>>>> + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
>>>>>>> +}
>>>>>>> +
>>>>>>> static void intel_disable_dp(struct intel_encoder *encoder)
>>>>>>> {
>>>>>>> struct intel_dp *intel_dp =
>>>>>>> enc_to_intel_dp(&encoder->base);
>>>>>>> @@ -2340,7 +2347,9 @@ static void intel_disable_dp(struct
>>>>>>> intel_encoder *encoder)
>>>>>>> * ensure that we have vdd while we switch off the
>>>>>>> panel. */
>>>>>>> intel_edp_panel_vdd_on(intel_dp);
>>>>>>> intel_edp_backlight_off(intel_dp);
>>>>>>> - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>>>>>>> + /* Skip power down to keep downstream HPD working */
>>>>>>> + if (!intel_dp_has_downstream_hpd(intel_dp))
>>>>>>> + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>>>>>>> intel_edp_panel_off(intel_dp);
>>>>>>> /* disable the port before the pipe on g4x */
>>>>>>> @@ -4944,6 +4953,13 @@ intel_dp_hpd_pulse(struct
>>>>>>> intel_digital_port *intel_dig_port, bool long_hpd)
>>>>>>> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>>>>>>> intel_dp_check_link_status(intel_dp);
>>>>>>> drm_modeset_unlock(&dev->mode_config.connection_mutex);
>>>>>>> +
>>>>>>> + /*
>>>>>>> + * Downstream HPD will generate a short HPD,
>>>>>>> + * so we want full hotplug processing here.
>>>>>>> + */
>>>>>>> + if (intel_dp_has_downstream_hpd(intel_dp))
>>>>>>> + goto put_power;
>>>>>>> }
>>>>>>> }
>>>>>> I am looking into compliance changes for DP and this seems a
>>>>>> relevant
>>>>>> change for compliance as well. but as per Link CTS 1.2 section
>>>>>> 4.2.2.8,
>>>>>> we are supposed to read the sink_count and do full detection if
>>>>>> sink_count is >1. So instead of checking for DP_DS_PORT_HPD can
>>>>>> we just
>>>>>> check SINK_COUNT and do full detect ?
>>>>> ->detect() will be called from the hotplug work and that will
>>>>> check SINK_COUNT.
>>>>>
>>>> No, the Compliance Sink tool, will not set the DP_DS_PORT_HPD
>>>> resulting
>>>> in detect not getting executed for
>>>> the short pulse generated. The spec requires the sink to set only the
>>>> sink count so it is not a must for
>>>> the sink to update the DP_DOWNSTREAM_PORT_0. so only a check for
>>>> SINK_COUNT will pass the
>>>> compliance test.
>>> That seems stupid. If the downstream port isn't HPD capable then we
>>> have
>>> no reason to check SINK_COUNT after a short HPD as the short HPD
>>> coudln't have been caused by a downstram HPD. Obviuously we still
>>> check SINK_COUNT after a long HPD to figure out if anything is
>>> connected
>>> when the branch device itself gets connected to the source.
>> Actually that's not correct. We don't check SINK_COUNT unless the
>> downstream
>> port is HPD capable.
>>
>> The spec says:
>> "If the DFP does not provide for means for plug/unplug detection, the
>> adaptor must set the SINK_COUNT field bits, as if those Sink devices
>> were
>> all permanently plugged."
>>
>> So according to the there can't be any changes in SINK_COUNT if the
>> downstream port is not HPD capable.
>>
>>
>>
> yes, agree on the no changes for SINK_COUNT if HPD is 0. i'll check
> with DP Compliance test
> tomorrow and confirm the exact reason for its failure may be my
> understanding of it was incorrect.
>
confirmed that the compliance sink is not setting HPD bit during detect.
so this looks to be a bug in
the sink tool. i'll file a bug with their team instead.
coming back to this patch, i will get back once i understand the complex
scenario of all short pulse
is treated as long pulse post this change, for example: we will do full
detection even if the sink requested
retraining of link.
--
regards,
Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Improve DP downstream HPD handling
2015-07-08 12:20 ` Sivakumar Thulasimani
@ 2015-07-08 12:27 ` Ville Syrjälä
0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2015-07-08 12:27 UTC (permalink / raw)
To: Sivakumar Thulasimani; +Cc: intel-gfx
On Wed, Jul 08, 2015 at 05:50:05PM +0530, Sivakumar Thulasimani wrote:
>
>
> On 7/7/2015 5:50 PM, Sivakumar Thulasimani wrote:
> >
> >
> > On 7/7/2015 5:24 PM, Ville Syrjälä wrote:
> >> On Tue, Jul 07, 2015 at 02:37:46PM +0300, Ville Syrjälä wrote:
> >>> On Tue, Jul 07, 2015 at 04:45:11PM +0530, Sivakumar Thulasimani wrote:
> >>>>
> >>>> On 7/7/2015 4:40 PM, Ville Syrjälä wrote:
> >>>>> On Tue, Jul 07, 2015 at 03:26:36PM +0530, Sivakumar Thulasimani
> >>>>> wrote:
> >>>>>> On 7/6/2015 5:42 PM, ville.syrjala@linux.intel.com wrote:
> >>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>>>
> >>>>>>> DP dongles may signal downstream HPD via short HPD pulses. If we
> >>>>>>> know
> >>>>>>> the device has a HPD capable downstream port, make sure we kick
> >>>>>>> off the
> >>>>>>> full hotplug processing even for short HPDs.
> >>>>>>>
> >>>>>>> Additonally setting the sink to DPMS off kills the downstream
> >>>>>>> HPD (at
> >>>>>>> least on my DP->VGA dongle), so skip the DPMS off for such dongles
> >>>>>>> when we turn off the port.
> >>>>>>>
> >>>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>>> ---
> >>>>>>> drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
> >>>>>>> 1 file changed, 17 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> >>>>>>> b/drivers/gpu/drm/i915/intel_dp.c
> >>>>>>> index e88cec2..f424833 100644
> >>>>>>> --- a/drivers/gpu/drm/i915/intel_dp.c
> >>>>>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >>>>>>> @@ -2324,6 +2324,13 @@ static void intel_dp_get_config(struct
> >>>>>>> intel_encoder *encoder,
> >>>>>>> }
> >>>>>>> }
> >>>>>>> +static bool intel_dp_has_downstream_hpd(struct intel_dp
> >>>>>>> *intel_dp)
> >>>>>>> +{
> >>>>>>> + return intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> >>>>>>> DP_DWN_STRM_PORT_PRESENT &&
> >>>>>>> + intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
> >>>>>>> + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> static void intel_disable_dp(struct intel_encoder *encoder)
> >>>>>>> {
> >>>>>>> struct intel_dp *intel_dp =
> >>>>>>> enc_to_intel_dp(&encoder->base);
> >>>>>>> @@ -2340,7 +2347,9 @@ static void intel_disable_dp(struct
> >>>>>>> intel_encoder *encoder)
> >>>>>>> * ensure that we have vdd while we switch off the
> >>>>>>> panel. */
> >>>>>>> intel_edp_panel_vdd_on(intel_dp);
> >>>>>>> intel_edp_backlight_off(intel_dp);
> >>>>>>> - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> >>>>>>> + /* Skip power down to keep downstream HPD working */
> >>>>>>> + if (!intel_dp_has_downstream_hpd(intel_dp))
> >>>>>>> + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> >>>>>>> intel_edp_panel_off(intel_dp);
> >>>>>>> /* disable the port before the pipe on g4x */
> >>>>>>> @@ -4944,6 +4953,13 @@ intel_dp_hpd_pulse(struct
> >>>>>>> intel_digital_port *intel_dig_port, bool long_hpd)
> >>>>>>> drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> >>>>>>> intel_dp_check_link_status(intel_dp);
> >>>>>>> drm_modeset_unlock(&dev->mode_config.connection_mutex);
> >>>>>>> +
> >>>>>>> + /*
> >>>>>>> + * Downstream HPD will generate a short HPD,
> >>>>>>> + * so we want full hotplug processing here.
> >>>>>>> + */
> >>>>>>> + if (intel_dp_has_downstream_hpd(intel_dp))
> >>>>>>> + goto put_power;
> >>>>>>> }
> >>>>>>> }
> >>>>>> I am looking into compliance changes for DP and this seems a
> >>>>>> relevant
> >>>>>> change for compliance as well. but as per Link CTS 1.2 section
> >>>>>> 4.2.2.8,
> >>>>>> we are supposed to read the sink_count and do full detection if
> >>>>>> sink_count is >1. So instead of checking for DP_DS_PORT_HPD can
> >>>>>> we just
> >>>>>> check SINK_COUNT and do full detect ?
> >>>>> ->detect() will be called from the hotplug work and that will
> >>>>> check SINK_COUNT.
> >>>>>
> >>>> No, the Compliance Sink tool, will not set the DP_DS_PORT_HPD
> >>>> resulting
> >>>> in detect not getting executed for
> >>>> the short pulse generated. The spec requires the sink to set only the
> >>>> sink count so it is not a must for
> >>>> the sink to update the DP_DOWNSTREAM_PORT_0. so only a check for
> >>>> SINK_COUNT will pass the
> >>>> compliance test.
> >>> That seems stupid. If the downstream port isn't HPD capable then we
> >>> have
> >>> no reason to check SINK_COUNT after a short HPD as the short HPD
> >>> coudln't have been caused by a downstram HPD. Obviuously we still
> >>> check SINK_COUNT after a long HPD to figure out if anything is
> >>> connected
> >>> when the branch device itself gets connected to the source.
> >> Actually that's not correct. We don't check SINK_COUNT unless the
> >> downstream
> >> port is HPD capable.
> >>
> >> The spec says:
> >> "If the DFP does not provide for means for plug/unplug detection, the
> >> adaptor must set the SINK_COUNT field bits, as if those Sink devices
> >> were
> >> all permanently plugged."
> >>
> >> So according to the there can't be any changes in SINK_COUNT if the
> >> downstream port is not HPD capable.
> >>
> >>
> >>
> > yes, agree on the no changes for SINK_COUNT if HPD is 0. i'll check
> > with DP Compliance test
> > tomorrow and confirm the exact reason for its failure may be my
> > understanding of it was incorrect.
> >
> confirmed that the compliance sink is not setting HPD bit during detect.
> so this looks to be a bug in
> the sink tool. i'll file a bug with their team instead.
>
> coming back to this patch, i will get back once i understand the complex
> scenario of all short pulse
> is treated as long pulse post this change, for example: we will do full
> detection even if the sink requested
> retraining of link.
I suppose we should read the reason for the short HPD from some DPCD
register, assuming one exists. I've not trawled the spec for such a
thing so can't say for sure.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] drm/i915: Improve DP downstream HPD handling
@ 2017-10-26 19:41 Ville Syrjala
2017-10-26 20:05 ` ✗ Fi.CI.BAT: failure for " Patchwork
` (5 more replies)
0 siblings, 6 replies; 20+ messages in thread
From: Ville Syrjala @ 2017-10-26 19:41 UTC (permalink / raw)
To: intel-gfx; +Cc: David Woodhouse, Pablo
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
DP dongles may signal downstream HPD via short HPD pulses. Setting the
sink to DPMS off apparently kills the downstream HPD (at least on my
DP->VGA dongle), so skip the DPMS off for such dongles when we turn
off the port.
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Pablo <pablodebiase@nanalysis.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103472
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99114
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index aa75f55eeb61..6ecc1b532f15 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2673,6 +2673,21 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
}
}
+static bool downstream_hpd_needs_d0(struct intel_dp *intel_dp)
+{
+ /*
+ * DPCD 1.2 should support BRANCH_DEVICE_CTRL, and thus
+ * be capable of signalling downstream hpd with a long pulse.
+ * Whether or not that means D3 is safe to use is not clear,
+ * but let's assume so until proven otherwise.
+ *
+ * FIXME should really check all downstream ports...
+ */
+ return intel_dp->dpcd[DP_DPCD_REV] == 0x11 &&
+ intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
+ intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
+}
+
static void intel_disable_dp(struct intel_encoder *encoder,
const struct intel_crtc_state *old_crtc_state,
const struct drm_connector_state *old_conn_state)
@@ -2686,7 +2701,8 @@ static void intel_disable_dp(struct intel_encoder *encoder,
* ensure that we have vdd while we switch off the panel. */
intel_edp_panel_vdd_on(intel_dp);
intel_edp_backlight_off(old_conn_state);
- intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
+ if (!downstream_hpd_needs_d0(intel_dp))
+ intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
intel_edp_panel_off(intel_dp);
}
--
2.13.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Improve DP downstream HPD handling
2017-10-26 19:41 [PATCH] drm/i915: Improve DP downstream HPD handling Ville Syrjala
@ 2017-10-26 20:05 ` Patchwork
2017-10-27 0:48 ` [PATCH] " Pandiyan, Dhinakaran
` (4 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-10-26 20:05 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Improve DP downstream HPD handling
URL : https://patchwork.freedesktop.org/series/32714/
State : failure
== Summary ==
Series 32714v1 drm/i915: Improve DP downstream HPD handling
https://patchwork.freedesktop.org/api/1.0/series/32714/revisions/1/mbox/
Test kms_busy:
Subgroup basic-flip-a:
pass -> INCOMPLETE (fi-glk-dsi)
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:439s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:448s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:372s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:520s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:264s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:496s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:490s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:489s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:478s
fi-cfl-s total:289 pass:253 dwarn:4 dfail:0 fail:0 skip:32 time:554s
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:601s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:416s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:571s
fi-glk-dsi total:206 pass:183 dwarn:0 dfail:0 fail:0 skip:22
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:431s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:434s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:440s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:504s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:456s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:487s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:573s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:477s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:580s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:542s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:453s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:594s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:649s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:521s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:507s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:460s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:561s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:420s
fi-gdg-551 failed to connect after reboot
faa3689f622af5d0e2d6bf106c84ab1d5ec49959 drm-tip: 2017y-10m-26d-18h-35m-28s UTC integration manifest
69697c0d3e79 drm/i915: Improve DP downstream HPD handling
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6213/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Improve DP downstream HPD handling
2017-10-26 19:41 [PATCH] drm/i915: Improve DP downstream HPD handling Ville Syrjala
2017-10-26 20:05 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2017-10-27 0:48 ` Pandiyan, Dhinakaran
2017-10-27 9:39 ` Ville Syrjälä
2017-10-27 9:45 ` [PATCH v2] " Ville Syrjala
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-10-27 0:48 UTC (permalink / raw)
To: ville.syrjala@linux.intel.com
Cc: intel-gfx@lists.freedesktop.org, dwmw2@infradead.org,
pablodebiase@nanalysis.com
On Thu, 2017-10-26 at 22:41 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> DP dongles may signal downstream HPD via short HPD pulses. Setting the
> sink to DPMS off apparently kills the downstream HPD (at least on my
> DP->VGA dongle), so skip the DPMS off for such dongles when we turn
> off the port.
>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Pablo <pablodebiase@nanalysis.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103472
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99114
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index aa75f55eeb61..6ecc1b532f15 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2673,6 +2673,21 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> }
> }
>
> +static bool downstream_hpd_needs_d0(struct intel_dp *intel_dp)
> +{
> + /*
> + * DPCD 1.2 should support BRANCH_DEVICE_CTRL, and thus
> + * be capable of signalling downstream hpd with a long pulse.
> + * Whether or not that means D3 is safe to use is not clear,
> + * but let's assume so until proven otherwise.
> + *
> + * FIXME should really check all downstream ports...
> + */
> + return intel_dp->dpcd[DP_DPCD_REV] == 0x11 &&
> + intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
> + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
> +}
> +
> static void intel_disable_dp(struct intel_encoder *encoder,
> const struct intel_crtc_state *old_crtc_state,
> const struct drm_connector_state *old_conn_state)
> @@ -2686,7 +2701,8 @@ static void intel_disable_dp(struct intel_encoder *encoder,
It's not clear why the DDI counterpart is missing.
> * ensure that we have vdd while we switch off the panel. */
> intel_edp_panel_vdd_on(intel_dp);
> intel_edp_backlight_off(old_conn_state);
> - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> + if (!downstream_hpd_needs_d0(intel_dp))
> + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> intel_edp_panel_off(intel_dp);
> }
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/i915: Improve DP downstream HPD handling
2017-10-27 0:48 ` [PATCH] " Pandiyan, Dhinakaran
@ 2017-10-27 9:39 ` Ville Syrjälä
0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-10-27 9:39 UTC (permalink / raw)
To: Pandiyan, Dhinakaran
Cc: intel-gfx@lists.freedesktop.org, dwmw2@infradead.org,
pablodebiase@nanalysis.com
On Fri, Oct 27, 2017 at 12:48:39AM +0000, Pandiyan, Dhinakaran wrote:
>
> On Thu, 2017-10-26 at 22:41 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > DP dongles may signal downstream HPD via short HPD pulses. Setting the
> > sink to DPMS off apparently kills the downstream HPD (at least on my
> > DP->VGA dongle), so skip the DPMS off for such dongles when we turn
> > off the port.
> >
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Pablo <pablodebiase@nanalysis.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103472
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99114
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index aa75f55eeb61..6ecc1b532f15 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2673,6 +2673,21 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
> > }
> > }
> >
> > +static bool downstream_hpd_needs_d0(struct intel_dp *intel_dp)
> > +{
> > + /*
> > + * DPCD 1.2 should support BRANCH_DEVICE_CTRL, and thus
> > + * be capable of signalling downstream hpd with a long pulse.
> > + * Whether or not that means D3 is safe to use is not clear,
> > + * but let's assume so until proven otherwise.
> > + *
> > + * FIXME should really check all downstream ports...
> > + */
> > + return intel_dp->dpcd[DP_DPCD_REV] == 0x11 &&
> > + intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
> > + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
> > +}
> > +
> > static void intel_disable_dp(struct intel_encoder *encoder,
> > const struct intel_crtc_state *old_crtc_state,
> > const struct drm_connector_state *old_conn_state)
> > @@ -2686,7 +2701,8 @@ static void intel_disable_dp(struct intel_encoder *encoder,
>
> It's not clear why the DDI counterpart is missing.
Because I forgot we have the same code in two places ;) v2 coming up...
>
> > * ensure that we have vdd while we switch off the panel. */
> > intel_edp_panel_vdd_on(intel_dp);
> > intel_edp_backlight_off(old_conn_state);
> > - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> > + if (!downstream_hpd_needs_d0(intel_dp))
> > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> > intel_edp_panel_off(intel_dp);
> > }
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] drm/i915: Improve DP downstream HPD handling
2017-10-26 19:41 [PATCH] drm/i915: Improve DP downstream HPD handling Ville Syrjala
2017-10-26 20:05 ` ✗ Fi.CI.BAT: failure for " Patchwork
2017-10-27 0:48 ` [PATCH] " Pandiyan, Dhinakaran
@ 2017-10-27 9:45 ` Ville Syrjala
2017-10-27 10:02 ` Jani Nikula
2017-10-27 10:22 ` ✗ Fi.CI.BAT: warning for drm/i915: Improve DP downstream HPD handling (rev2) Patchwork
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjala @ 2017-10-27 9:45 UTC (permalink / raw)
To: intel-gfx; +Cc: Pablo, David Woodhouse, Dhinakaran Pandiyan
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
DP dongles may signal downstream HPD via short HPD pulses. Setting the
sink to DPMS off apparently kills the downstream HPD (at least on my
DP->VGA dongle), so skip the DPMS off for such dongles when we turn
off the port.
v2: Deal with DDI as well by moving the check into
intel_dp_sink_dpms() (Dhinakaran)
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Pablo <pablodebiase@nanalysis.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103472
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99114
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index aa75f55eeb61..59169a4dc9d3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2505,6 +2505,21 @@ static void ironlake_edp_pll_off(struct intel_dp *intel_dp)
udelay(200);
}
+static bool downstream_hpd_needs_d0(struct intel_dp *intel_dp)
+{
+ /*
+ * DPCD 1.2+ should support BRANCH_DEVICE_CTRL, and thus
+ * be capable of signalling downstream hpd with a long pulse.
+ * Whether or not that means D3 is safe to use is not clear,
+ * but let's assume so until proven otherwise.
+ *
+ * FIXME should really check all downstream ports...
+ */
+ return intel_dp->dpcd[DP_DPCD_REV] == 0x11 &&
+ intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
+ intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
+}
+
/* If the sink supports it, try to set the power state appropriately */
void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
{
@@ -2515,6 +2530,9 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
return;
if (mode != DRM_MODE_DPMS_ON) {
+ if (downstream_hpd_needs_d0(intel_dp))
+ return;
+
ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
DP_SET_POWER_D3);
} else {
--
2.13.6
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: Improve DP downstream HPD handling
2017-10-27 9:45 ` [PATCH v2] " Ville Syrjala
@ 2017-10-27 10:02 ` Jani Nikula
2017-10-27 19:34 ` Ville Syrjälä
0 siblings, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2017-10-27 10:02 UTC (permalink / raw)
To: Ville Syrjala, intel-gfx; +Cc: Pablo, David Woodhouse, Dhinakaran Pandiyan
On Fri, 27 Oct 2017, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> DP dongles may signal downstream HPD via short HPD pulses. Setting the
> sink to DPMS off apparently kills the downstream HPD (at least on my
> DP->VGA dongle), so skip the DPMS off for such dongles when we turn
> off the port.
>
> v2: Deal with DDI as well by moving the check into
> intel_dp_sink_dpms() (Dhinakaran)
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Pablo <pablodebiase@nanalysis.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103472
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99114
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index aa75f55eeb61..59169a4dc9d3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2505,6 +2505,21 @@ static void ironlake_edp_pll_off(struct intel_dp *intel_dp)
> udelay(200);
> }
>
> +static bool downstream_hpd_needs_d0(struct intel_dp *intel_dp)
> +{
> + /*
> + * DPCD 1.2+ should support BRANCH_DEVICE_CTRL, and thus
> + * be capable of signalling downstream hpd with a long pulse.
> + * Whether or not that means D3 is safe to use is not clear,
> + * but let's assume so until proven otherwise.
> + *
> + * FIXME should really check all downstream ports...
> + */
> + return intel_dp->dpcd[DP_DPCD_REV] == 0x11 &&
> + intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
> + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
> +}
> +
> /* If the sink supports it, try to set the power state appropriately */
> void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> {
> @@ -2515,6 +2530,9 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> return;
>
> if (mode != DRM_MODE_DPMS_ON) {
> + if (downstream_hpd_needs_d0(intel_dp))
> + return;
> +
> ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D3);
> } else {
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915: Improve DP downstream HPD handling (rev2)
2017-10-26 19:41 [PATCH] drm/i915: Improve DP downstream HPD handling Ville Syrjala
` (2 preceding siblings ...)
2017-10-27 9:45 ` [PATCH v2] " Ville Syrjala
@ 2017-10-27 10:22 ` Patchwork
2017-10-27 13:03 ` ✓ Fi.CI.BAT: success " Patchwork
2017-10-27 15:50 ` ✓ Fi.CI.IGT: " Patchwork
5 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-10-27 10:22 UTC (permalink / raw)
To: Ville Syrjala; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Improve DP downstream HPD handling (rev2)
URL : https://patchwork.freedesktop.org/series/32714/
State : warning
== Summary ==
Series 32714v2 drm/i915: Improve DP downstream HPD handling
https://patchwork.freedesktop.org/api/1.0/series/32714/revisions/2/mbox/
Test gem_exec_reloc:
Subgroup basic-cpu-gtt-active:
pass -> FAIL (fi-gdg-551) fdo#102582 +2
Test gem_exec_suspend:
Subgroup basic-s3:
dmesg-warn -> PASS (fi-cfl-s) fdo#103186
Test gem_ringfill:
Subgroup basic-default-hang:
dmesg-warn -> PASS (fi-pnv-d510) fdo#101600
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-WARN (fi-byt-n2820) fdo#101705
Test prime_vgem:
Subgroup basic-fence-mmap:
pass -> DMESG-WARN (fi-bsw-n3050)
fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fdo#103186 https://bugs.freedesktop.org/show_bug.cgi?id=103186
fdo#101600 https://bugs.freedesktop.org/show_bug.cgi?id=101600
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:441s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:454s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:370s
fi-bsw-n3050 total:289 pass:242 dwarn:1 dfail:0 fail:0 skip:46 time:530s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:262s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:499s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:492s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:494s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:478s
fi-cfl-s total:289 pass:254 dwarn:3 dfail:0 fail:0 skip:32 time:559s
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:608s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:422s
fi-gdg-551 total:289 pass:175 dwarn:1 dfail:0 fail:4 skip:109 time:251s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:579s
fi-glk-dsi total:289 pass:258 dwarn:0 dfail:0 fail:1 skip:30 time:493s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:431s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:430s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:430s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:492s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:467s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:494s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:574s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:475s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:585s
fi-pnv-d510 total:289 pass:223 dwarn:0 dfail:0 fail:0 skip:66 time:546s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:451s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:593s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:648s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:521s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:504s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:456s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:560s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:419s
1db1a27d38d789ce2886db512828ae306c998bc3 drm-tip: 2017y-10m-27d-07h-23m-09s UTC integration manifest
7793aaa4f221 drm/i915: Improve DP downstream HPD handling
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6225/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Improve DP downstream HPD handling (rev2)
2017-10-26 19:41 [PATCH] drm/i915: Improve DP downstream HPD handling Ville Syrjala
` (3 preceding siblings ...)
2017-10-27 10:22 ` ✗ Fi.CI.BAT: warning for drm/i915: Improve DP downstream HPD handling (rev2) Patchwork
@ 2017-10-27 13:03 ` Patchwork
2017-10-27 15:50 ` ✓ Fi.CI.IGT: " Patchwork
5 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-10-27 13:03 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Improve DP downstream HPD handling (rev2)
URL : https://patchwork.freedesktop.org/series/32714/
State : success
== Summary ==
Series 32714v2 drm/i915: Improve DP downstream HPD handling
https://patchwork.freedesktop.org/api/1.0/series/32714/revisions/2/mbox/
Test chamelium:
Subgroup dp-crc-fast:
pass -> FAIL (fi-kbl-7500u) fdo#102514
Test gem_exec_reloc:
Subgroup basic-cpu-active:
pass -> FAIL (fi-gdg-551) fdo#102582 +2
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
fail -> PASS (fi-gdg-551) fdo#102618
fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:440s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:451s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:369s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:530s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:263s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:501s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:498s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:494s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:476s
fi-cnl-y total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:595s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:411s
fi-gdg-551 total:289 pass:175 dwarn:1 dfail:0 fail:4 skip:109 time:251s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:580s
fi-glk-dsi total:289 pass:258 dwarn:0 dfail:0 fail:1 skip:30 time:489s
fi-hsw-4770 total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:427s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:432s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:432s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:484s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:460s
fi-kbl-7500u total:289 pass:263 dwarn:1 dfail:0 fail:1 skip:24 time:482s
fi-kbl-7560u total:289 pass:270 dwarn:0 dfail:0 fail:0 skip:19 time:572s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:482s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:579s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:545s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:451s
fi-skl-6600u total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:591s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:642s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:517s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:506s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:453s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:563s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:416s
795d258e67f403ff77d1f14b37551e98163f5b4e drm-tip: 2017y-10m-27d-11h-42m-39s UTC integration manifest
e056a2eee73b drm/i915: Improve DP downstream HPD handling
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6230/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: Improve DP downstream HPD handling (rev2)
2017-10-26 19:41 [PATCH] drm/i915: Improve DP downstream HPD handling Ville Syrjala
` (4 preceding siblings ...)
2017-10-27 13:03 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2017-10-27 15:50 ` Patchwork
5 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2017-10-27 15:50 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Improve DP downstream HPD handling (rev2)
URL : https://patchwork.freedesktop.org/series/32714/
State : success
== Summary ==
Test kms_busy:
Subgroup extended-modeset-hang-oldfb-with-reset-render-B:
pass -> DMESG-WARN (shard-hsw) fdo#102249 +1
fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
shard-hsw total:2539 pass:1432 dwarn:2 dfail:0 fail:8 skip:1097 time:9176s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6230/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/i915: Improve DP downstream HPD handling
2017-10-27 10:02 ` Jani Nikula
@ 2017-10-27 19:34 ` Ville Syrjälä
0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2017-10-27 19:34 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx, David Woodhouse, Pablo, Dhinakaran Pandiyan
On Fri, Oct 27, 2017 at 01:02:24PM +0300, Jani Nikula wrote:
> On Fri, 27 Oct 2017, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > DP dongles may signal downstream HPD via short HPD pulses. Setting the
> > sink to DPMS off apparently kills the downstream HPD (at least on my
> > DP->VGA dongle), so skip the DPMS off for such dongles when we turn
> > off the port.
> >
> > v2: Deal with DDI as well by moving the check into
> > intel_dp_sink_dpms() (Dhinakaran)
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Pablo <pablodebiase@nanalysis.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103472
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99114
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Thanks for the review. Patch pushed to dinq.
>
>
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index aa75f55eeb61..59169a4dc9d3 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2505,6 +2505,21 @@ static void ironlake_edp_pll_off(struct intel_dp *intel_dp)
> > udelay(200);
> > }
> >
> > +static bool downstream_hpd_needs_d0(struct intel_dp *intel_dp)
> > +{
> > + /*
> > + * DPCD 1.2+ should support BRANCH_DEVICE_CTRL, and thus
> > + * be capable of signalling downstream hpd with a long pulse.
> > + * Whether or not that means D3 is safe to use is not clear,
> > + * but let's assume so until proven otherwise.
> > + *
> > + * FIXME should really check all downstream ports...
> > + */
> > + return intel_dp->dpcd[DP_DPCD_REV] == 0x11 &&
> > + intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT &&
> > + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD;
> > +}
> > +
> > /* If the sink supports it, try to set the power state appropriately */
> > void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> > {
> > @@ -2515,6 +2530,9 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
> > return;
> >
> > if (mode != DRM_MODE_DPMS_ON) {
> > + if (downstream_hpd_needs_d0(intel_dp))
> > + return;
> > +
> > ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D3);
> > } else {
>
> --
> Jani Nikula, Intel Open Source Technology Center
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-10-27 19:34 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-26 19:41 [PATCH] drm/i915: Improve DP downstream HPD handling Ville Syrjala
2017-10-26 20:05 ` ✗ Fi.CI.BAT: failure for " Patchwork
2017-10-27 0:48 ` [PATCH] " Pandiyan, Dhinakaran
2017-10-27 9:39 ` Ville Syrjälä
2017-10-27 9:45 ` [PATCH v2] " Ville Syrjala
2017-10-27 10:02 ` Jani Nikula
2017-10-27 19:34 ` Ville Syrjälä
2017-10-27 10:22 ` ✗ Fi.CI.BAT: warning for drm/i915: Improve DP downstream HPD handling (rev2) Patchwork
2017-10-27 13:03 ` ✓ Fi.CI.BAT: success " Patchwork
2017-10-27 15:50 ` ✓ Fi.CI.IGT: " Patchwork
-- strict thread matches above, loose matches on Subject: below --
2015-07-06 12:12 [PATCH] drm/i915: Improve DP downstream HPD handling ville.syrjala
2015-07-07 9:56 ` Sivakumar Thulasimani
2015-07-07 11:10 ` Ville Syrjälä
2015-07-07 11:15 ` Sivakumar Thulasimani
2015-07-07 11:37 ` Ville Syrjälä
2015-07-07 11:54 ` Ville Syrjälä
2015-07-07 12:20 ` Sivakumar Thulasimani
2015-07-08 12:20 ` Sivakumar Thulasimani
2015-07-08 12:27 ` Ville Syrjälä
2015-07-07 11:23 ` shuang.he
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox