| Message ID | 1439811096-13561-2-git-send-email-sivakumar.thulasimani@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 17 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote: > From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com> > > Compliance test 4.2.2.8 requires driver to read the sink_count for > short pulse interrupt even when the panel is not enabled. > This patch performs the following > a) reading sink_count by reusing intel_dp_detect_dpcd > instead of using intel_dp_get_dpcd > b) moving crtc enabled checks post sink_count read call > > Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 117 ++++++++++++++++++++------------------- > 1 file changed, 59 insertions(+), 58 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index b905c19..e4de8e5 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -4342,6 +4342,56 @@ go_again: > return -EINVAL; > } > > +/* XXX this is probably wrong for multiple downstream ports */ > +static enum drm_connector_status > +intel_dp_detect_dpcd(struct intel_dp *intel_dp) > +{ > + uint8_t *dpcd = intel_dp->dpcd; > + uint8_t type; > + > + if (!intel_dp_get_dpcd(intel_dp)) > + return connector_status_disconnected; > + > + /* if there's no downstream port, we're done */ > + if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT)) > + return connector_status_connected; > + > + /* If we're HPD-aware, SINK_COUNT changes dynamically */ > + if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && > + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { > + uint8_t reg; > + > + if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, > + ®, 1) < 0) > + return connector_status_unknown; > + > + return DP_GET_SINK_COUNT(reg) ? connector_status_connected > + : connector_status_disconnected; > + } > + > + /* If no HPD, poke DDC gently */ > + if (drm_probe_ddc(&intel_dp->aux.ddc)) > + return connector_status_connected; > + > + /* Well we tried, say unknown for unreliable port types */ > + if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) { > + type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK; > + if (type == DP_DS_PORT_TYPE_VGA || > + type == DP_DS_PORT_TYPE_NON_EDID) > + return connector_status_unknown; > + } else { > + type = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & > + DP_DWN_STRM_PORT_TYPE_MASK; > + if (type == DP_DWN_STRM_PORT_TYPE_ANALOG || > + type == DP_DWN_STRM_PORT_TYPE_OTHER) > + return connector_status_unknown; > + } > + > + /* Anything else is out of spec, warn and ignore */ > + DRM_DEBUG_KMS("Broken DP branch device, ignoring\n"); > + return connector_status_disconnected; > +} Please either a) just add a forward declaration for intel_dp_detect_dpcd, or b) add a separate non-functional prep patch that moves the function around. Please don't combine code movement with functional changes. BR, Jani. > + > /* > * According to DP spec > * 5.1.2: > @@ -4362,21 +4412,22 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) > > WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); > > - if (!intel_encoder->base.crtc) > + /* 4.2.2.8 requires source to read link_status, 0 - 12 DPCD & > + * sink_count even for short pulse irrespective of the sink is > + * in use or not > + */ > + if (!intel_dp_get_link_status(intel_dp, link_status)) > return; > > - if (!to_intel_crtc(intel_encoder->base.crtc)->active) > + /* reuse to read both 0 - 12 DPCD & sink_count */ > + if (intel_dp_detect_dpcd(intel_dp) != connector_status_connected) > return; > > - /* Try to read receiver status if the link appears to be up */ > - if (!intel_dp_get_link_status(intel_dp, link_status)) { > + if (!intel_encoder->base.crtc) > return; > - } > > - /* Now read the DPCD to see if it's actually running */ > - if (!intel_dp_get_dpcd(intel_dp)) { > + if (!to_intel_crtc(intel_encoder->base.crtc)->active) > return; > - } > > /* Try to read the source of the interrupt */ > if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && > @@ -4401,56 +4452,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) > } > } > > -/* XXX this is probably wrong for multiple downstream ports */ > -static enum drm_connector_status > -intel_dp_detect_dpcd(struct intel_dp *intel_dp) > -{ > - uint8_t *dpcd = intel_dp->dpcd; > - uint8_t type; > - > - if (!intel_dp_get_dpcd(intel_dp)) > - return connector_status_disconnected; > - > - /* if there's no downstream port, we're done */ > - if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT)) > - return connector_status_connected; > - > - /* If we're HPD-aware, SINK_COUNT changes dynamically */ > - if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && > - intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { > - uint8_t reg; > - > - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, > - ®, 1) < 0) > - return connector_status_unknown; > - > - return DP_GET_SINK_COUNT(reg) ? connector_status_connected > - : connector_status_disconnected; > - } > - > - /* If no HPD, poke DDC gently */ > - if (drm_probe_ddc(&intel_dp->aux.ddc)) > - return connector_status_connected; > - > - /* Well we tried, say unknown for unreliable port types */ > - if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) { > - type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK; > - if (type == DP_DS_PORT_TYPE_VGA || > - type == DP_DS_PORT_TYPE_NON_EDID) > - return connector_status_unknown; > - } else { > - type = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & > - DP_DWN_STRM_PORT_TYPE_MASK; > - if (type == DP_DWN_STRM_PORT_TYPE_ANALOG || > - type == DP_DWN_STRM_PORT_TYPE_OTHER) > - return connector_status_unknown; > - } > - > - /* Anything else is out of spec, warn and ignore */ > - DRM_DEBUG_KMS("Broken DP branch device, ignoring\n"); > - return connector_status_disconnected; > -} > - > static enum drm_connector_status > edp_detect(struct intel_dp *intel_dp) > { > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 8/17/2015 5:39 PM, Jani Nikula wrote: > On Mon, 17 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote: >> From: "Thulasimani,Sivakumar" <sivakumar.thulasimani@intel.com> >> >> Compliance test 4.2.2.8 requires driver to read the sink_count for >> short pulse interrupt even when the panel is not enabled. >> This patch performs the following >> a) reading sink_count by reusing intel_dp_detect_dpcd >> instead of using intel_dp_get_dpcd >> b) moving crtc enabled checks post sink_count read call >> >> Signed-off-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 117 ++++++++++++++++++++------------------- >> 1 file changed, 59 insertions(+), 58 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index b905c19..e4de8e5 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -4342,6 +4342,56 @@ go_again: >> return -EINVAL; >> } >> >> +/* XXX this is probably wrong for multiple downstream ports */ >> +static enum drm_connector_status >> +intel_dp_detect_dpcd(struct intel_dp *intel_dp) >> +{ >> + uint8_t *dpcd = intel_dp->dpcd; >> + uint8_t type; >> + >> + if (!intel_dp_get_dpcd(intel_dp)) >> + return connector_status_disconnected; >> + >> + /* if there's no downstream port, we're done */ >> + if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT)) >> + return connector_status_connected; >> + >> + /* If we're HPD-aware, SINK_COUNT changes dynamically */ >> + if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && >> + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { >> + uint8_t reg; >> + >> + if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, >> + ®, 1) < 0) >> + return connector_status_unknown; >> + >> + return DP_GET_SINK_COUNT(reg) ? connector_status_connected >> + : connector_status_disconnected; >> + } >> + >> + /* If no HPD, poke DDC gently */ >> + if (drm_probe_ddc(&intel_dp->aux.ddc)) >> + return connector_status_connected; >> + >> + /* Well we tried, say unknown for unreliable port types */ >> + if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) { >> + type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK; >> + if (type == DP_DS_PORT_TYPE_VGA || >> + type == DP_DS_PORT_TYPE_NON_EDID) >> + return connector_status_unknown; >> + } else { >> + type = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & >> + DP_DWN_STRM_PORT_TYPE_MASK; >> + if (type == DP_DWN_STRM_PORT_TYPE_ANALOG || >> + type == DP_DWN_STRM_PORT_TYPE_OTHER) >> + return connector_status_unknown; >> + } >> + >> + /* Anything else is out of spec, warn and ignore */ >> + DRM_DEBUG_KMS("Broken DP branch device, ignoring\n"); >> + return connector_status_disconnected; >> +} > Please either a) just add a forward declaration for > intel_dp_detect_dpcd, or b) add a separate non-functional prep patch > that moves the function around. Please don't combine code movement with > functional changes. > > BR, > Jani. uploaded V2 of the patch with forward declaration. > >> + >> /* >> * According to DP spec >> * 5.1.2: >> @@ -4362,21 +4412,22 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) >> >> WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); >> >> - if (!intel_encoder->base.crtc) >> + /* 4.2.2.8 requires source to read link_status, 0 - 12 DPCD & >> + * sink_count even for short pulse irrespective of the sink is >> + * in use or not >> + */ >> + if (!intel_dp_get_link_status(intel_dp, link_status)) >> return; >> >> - if (!to_intel_crtc(intel_encoder->base.crtc)->active) >> + /* reuse to read both 0 - 12 DPCD & sink_count */ >> + if (intel_dp_detect_dpcd(intel_dp) != connector_status_connected) >> return; >> >> - /* Try to read receiver status if the link appears to be up */ >> - if (!intel_dp_get_link_status(intel_dp, link_status)) { >> + if (!intel_encoder->base.crtc) >> return; >> - } >> >> - /* Now read the DPCD to see if it's actually running */ >> - if (!intel_dp_get_dpcd(intel_dp)) { >> + if (!to_intel_crtc(intel_encoder->base.crtc)->active) >> return; >> - } >> >> /* Try to read the source of the interrupt */ >> if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && >> @@ -4401,56 +4452,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) >> } >> } >> >> -/* XXX this is probably wrong for multiple downstream ports */ >> -static enum drm_connector_status >> -intel_dp_detect_dpcd(struct intel_dp *intel_dp) >> -{ >> - uint8_t *dpcd = intel_dp->dpcd; >> - uint8_t type; >> - >> - if (!intel_dp_get_dpcd(intel_dp)) >> - return connector_status_disconnected; >> - >> - /* if there's no downstream port, we're done */ >> - if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT)) >> - return connector_status_connected; >> - >> - /* If we're HPD-aware, SINK_COUNT changes dynamically */ >> - if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && >> - intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { >> - uint8_t reg; >> - >> - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, >> - ®, 1) < 0) >> - return connector_status_unknown; >> - >> - return DP_GET_SINK_COUNT(reg) ? connector_status_connected >> - : connector_status_disconnected; >> - } >> - >> - /* If no HPD, poke DDC gently */ >> - if (drm_probe_ddc(&intel_dp->aux.ddc)) >> - return connector_status_connected; >> - >> - /* Well we tried, say unknown for unreliable port types */ >> - if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) { >> - type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK; >> - if (type == DP_DS_PORT_TYPE_VGA || >> - type == DP_DS_PORT_TYPE_NON_EDID) >> - return connector_status_unknown; >> - } else { >> - type = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & >> - DP_DWN_STRM_PORT_TYPE_MASK; >> - if (type == DP_DWN_STRM_PORT_TYPE_ANALOG || >> - type == DP_DWN_STRM_PORT_TYPE_OTHER) >> - return connector_status_unknown; >> - } >> - >> - /* Anything else is out of spec, warn and ignore */ >> - DRM_DEBUG_KMS("Broken DP branch device, ignoring\n"); >> - return connector_status_disconnected; >> -} >> - >> static enum drm_connector_status >> edp_detect(struct intel_dp *intel_dp) >> { >> -- >> 1.7.9.5 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index b905c19..e4de8e5 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4342,6 +4342,56 @@ go_again: return -EINVAL; } +/* XXX this is probably wrong for multiple downstream ports */ +static enum drm_connector_status +intel_dp_detect_dpcd(struct intel_dp *intel_dp) +{ + uint8_t *dpcd = intel_dp->dpcd; + uint8_t type; + + if (!intel_dp_get_dpcd(intel_dp)) + return connector_status_disconnected; + + /* if there's no downstream port, we're done */ + if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT)) + return connector_status_connected; + + /* If we're HPD-aware, SINK_COUNT changes dynamically */ + if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && + intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { + uint8_t reg; + + if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, + ®, 1) < 0) + return connector_status_unknown; + + return DP_GET_SINK_COUNT(reg) ? connector_status_connected + : connector_status_disconnected; + } + + /* If no HPD, poke DDC gently */ + if (drm_probe_ddc(&intel_dp->aux.ddc)) + return connector_status_connected; + + /* Well we tried, say unknown for unreliable port types */ + if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) { + type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK; + if (type == DP_DS_PORT_TYPE_VGA || + type == DP_DS_PORT_TYPE_NON_EDID) + return connector_status_unknown; + } else { + type = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & + DP_DWN_STRM_PORT_TYPE_MASK; + if (type == DP_DWN_STRM_PORT_TYPE_ANALOG || + type == DP_DWN_STRM_PORT_TYPE_OTHER) + return connector_status_unknown; + } + + /* Anything else is out of spec, warn and ignore */ + DRM_DEBUG_KMS("Broken DP branch device, ignoring\n"); + return connector_status_disconnected; +} + /* * According to DP spec * 5.1.2: @@ -4362,21 +4412,22 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex)); - if (!intel_encoder->base.crtc) + /* 4.2.2.8 requires source to read link_status, 0 - 12 DPCD & + * sink_count even for short pulse irrespective of the sink is + * in use or not + */ + if (!intel_dp_get_link_status(intel_dp, link_status)) return; - if (!to_intel_crtc(intel_encoder->base.crtc)->active) + /* reuse to read both 0 - 12 DPCD & sink_count */ + if (intel_dp_detect_dpcd(intel_dp) != connector_status_connected) return; - /* Try to read receiver status if the link appears to be up */ - if (!intel_dp_get_link_status(intel_dp, link_status)) { + if (!intel_encoder->base.crtc) return; - } - /* Now read the DPCD to see if it's actually running */ - if (!intel_dp_get_dpcd(intel_dp)) { + if (!to_intel_crtc(intel_encoder->base.crtc)->active) return; - } /* Try to read the source of the interrupt */ if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && @@ -4401,56 +4452,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) } } -/* XXX this is probably wrong for multiple downstream ports */ -static enum drm_connector_status -intel_dp_detect_dpcd(struct intel_dp *intel_dp) -{ - uint8_t *dpcd = intel_dp->dpcd; - uint8_t type; - - if (!intel_dp_get_dpcd(intel_dp)) - return connector_status_disconnected; - - /* if there's no downstream port, we're done */ - if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT)) - return connector_status_connected; - - /* If we're HPD-aware, SINK_COUNT changes dynamically */ - if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 && - intel_dp->downstream_ports[0] & DP_DS_PORT_HPD) { - uint8_t reg; - - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, - ®, 1) < 0) - return connector_status_unknown; - - return DP_GET_SINK_COUNT(reg) ? connector_status_connected - : connector_status_disconnected; - } - - /* If no HPD, poke DDC gently */ - if (drm_probe_ddc(&intel_dp->aux.ddc)) - return connector_status_connected; - - /* Well we tried, say unknown for unreliable port types */ - if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) { - type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK; - if (type == DP_DS_PORT_TYPE_VGA || - type == DP_DS_PORT_TYPE_NON_EDID) - return connector_status_unknown; - } else { - type = intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] & - DP_DWN_STRM_PORT_TYPE_MASK; - if (type == DP_DWN_STRM_PORT_TYPE_ANALOG || - type == DP_DWN_STRM_PORT_TYPE_OTHER) - return connector_status_unknown; - } - - /* Anything else is out of spec, warn and ignore */ - DRM_DEBUG_KMS("Broken DP branch device, ignoring\n"); - return connector_status_disconnected; -} - static enum drm_connector_status edp_detect(struct intel_dp *intel_dp) {