| Message ID | 1462216105-20881-4-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 5/3/2016 12:38 AM, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > To save a bit of power, let's try to turn off the TMDS output buffers > in DP++ adaptors when we're not driving the port. > > v2: Let's not forget DDI, toss in a debug message while at it > v3: Just do the TMDS output control based on adaptor type. With the > helper getting passed the type, we wouldn't actually have to > check at all in the driver, but the check eliminates the debug > output more honest > > Cc: stable@vger.kernel.org > Cc: Tore Anderson <tore@fud.no> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Cc: Shashank Sharma <shashank.sharma@intel.com> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_ddi.c | 12 ++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_hdmi.c | 20 ++++++++++++++++++++ > 3 files changed, 33 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 422ec81ef59b..a3600704e6d4 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1601,6 +1601,12 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) > enum port port = intel_ddi_get_encoder_port(intel_encoder); > int type = intel_encoder->type; > > + if (type == INTEL_OUTPUT_HDMI) { how to handle type2 active adapters here, which can show type = DP ? > + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); > + > + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); > + } > + > intel_prepare_ddi_buffer(intel_encoder); > > if (type == INTEL_OUTPUT_EDP) { > @@ -1667,6 +1673,12 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder) > DPLL_CTRL2_DDI_CLK_OFF(port))); > else if (INTEL_INFO(dev)->gen < 9) > I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE); > + > + if (type == INTEL_OUTPUT_HDMI) { Same as above. Regards Shashank > + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); > + > + intel_dp_dual_mode_set_tmds_output(intel_hdmi, false); > + } > } > > static void intel_enable_ddi(struct intel_encoder *intel_encoder) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index e94d18fb2ff1..dbcb80c70e2e 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1419,6 +1419,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, > struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder); > bool intel_hdmi_compute_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config); > +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable); > > > /* intel_lvds.c */ > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index 31ca11134294..c4d93e6b4bed 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -836,6 +836,22 @@ static void hsw_set_infoframes(struct drm_encoder *encoder, > intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode); > } > > +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable) > +{ > + struct drm_i915_private *dev_priv = to_i915(intel_hdmi_to_dev(hdmi)); > + struct i2c_adapter *adapter = > + intel_gmbus_get_adapter(dev_priv, hdmi->ddc_bus); > + > + if (hdmi->dp_dual_mode.type < DRM_DP_DUAL_MODE_TYPE2_DVI) > + return; > + > + DRM_DEBUG_KMS("%s DP dual mode adaptor TMDS output\n", > + enable ? "Enabling" : "Disabling"); > + > + drm_dp_dual_mode_set_tmds_output(hdmi->dp_dual_mode.type, > + adapter, enable); > +} > + > static void intel_hdmi_prepare(struct intel_encoder *encoder) > { > struct drm_device *dev = encoder->base.dev; > @@ -845,6 +861,8 @@ static void intel_hdmi_prepare(struct intel_encoder *encoder) > const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode; > u32 hdmi_val; > > + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); > + > hdmi_val = SDVO_ENCODING_HDMI; > if (!HAS_PCH_SPLIT(dev) && crtc->config->limited_color_range) > hdmi_val |= HDMI_COLOR_RANGE_16_235; > @@ -1142,6 +1160,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder) > } > > intel_hdmi->set_infoframes(&encoder->base, false, NULL); > + > + intel_dp_dual_mode_set_tmds_output(intel_hdmi, false); > } > > static void g4x_disable_hdmi(struct intel_encoder *encoder) >
On Wed, May 04, 2016 at 03:43:30PM +0530, Sharma, Shashank wrote: > > On 5/3/2016 12:38 AM, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > To save a bit of power, let's try to turn off the TMDS output buffers > > in DP++ adaptors when we're not driving the port. > > > > v2: Let's not forget DDI, toss in a debug message while at it > > v3: Just do the TMDS output control based on adaptor type. With the > > helper getting passed the type, we wouldn't actually have to > > check at all in the driver, but the check eliminates the debug > > output more honest > > > > Cc: stable@vger.kernel.org > > Cc: Tore Anderson <tore@fud.no> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: Shashank Sharma <shashank.sharma@intel.com> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 12 ++++++++++++ > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > drivers/gpu/drm/i915/intel_hdmi.c | 20 ++++++++++++++++++++ > > 3 files changed, 33 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > index 422ec81ef59b..a3600704e6d4 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -1601,6 +1601,12 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) > > enum port port = intel_ddi_get_encoder_port(intel_encoder); > > int type = intel_encoder->type; > > > > + if (type == INTEL_OUTPUT_HDMI) { > how to handle type2 active adapters here, which can show type = DP ? The dual mode concept shouldn't apply to active adapters. Those, as you say, appear as DP and so will be handled as if they were native DP sinks more or less. Or are you aware of active DP dongles that also implement the dual mode standatd? Apart from LSPCON of course. Actually I'm not sure how LSPCON will handle this. If we drive it in PCON mode, would it still respect the dual mode register settings, including the TMDS OE# state? > > + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); > > + > > + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); > > + } > > + > > intel_prepare_ddi_buffer(intel_encoder); > > > > if (type == INTEL_OUTPUT_EDP) { > > @@ -1667,6 +1673,12 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder) > > DPLL_CTRL2_DDI_CLK_OFF(port))); > > else if (INTEL_INFO(dev)->gen < 9) > > I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE); > > + > > + if (type == INTEL_OUTPUT_HDMI) { > Same as above. > > Regards > Shashank > > + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); > > + > > + intel_dp_dual_mode_set_tmds_output(intel_hdmi, false); > > + } > > } > > > > static void intel_enable_ddi(struct intel_encoder *intel_encoder) > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index e94d18fb2ff1..dbcb80c70e2e 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1419,6 +1419,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, > > struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder); > > bool intel_hdmi_compute_config(struct intel_encoder *encoder, > > struct intel_crtc_state *pipe_config); > > +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable); > > > > > > /* intel_lvds.c */ > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > > index 31ca11134294..c4d93e6b4bed 100644 > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > @@ -836,6 +836,22 @@ static void hsw_set_infoframes(struct drm_encoder *encoder, > > intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode); > > } > > > > +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(intel_hdmi_to_dev(hdmi)); > > + struct i2c_adapter *adapter = > > + intel_gmbus_get_adapter(dev_priv, hdmi->ddc_bus); > > + > > + if (hdmi->dp_dual_mode.type < DRM_DP_DUAL_MODE_TYPE2_DVI) > > + return; > > + > > + DRM_DEBUG_KMS("%s DP dual mode adaptor TMDS output\n", > > + enable ? "Enabling" : "Disabling"); > > + > > + drm_dp_dual_mode_set_tmds_output(hdmi->dp_dual_mode.type, > > + adapter, enable); > > +} > > + > > static void intel_hdmi_prepare(struct intel_encoder *encoder) > > { > > struct drm_device *dev = encoder->base.dev; > > @@ -845,6 +861,8 @@ static void intel_hdmi_prepare(struct intel_encoder *encoder) > > const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode; > > u32 hdmi_val; > > > > + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); > > + > > hdmi_val = SDVO_ENCODING_HDMI; > > if (!HAS_PCH_SPLIT(dev) && crtc->config->limited_color_range) > > hdmi_val |= HDMI_COLOR_RANGE_16_235; > > @@ -1142,6 +1160,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder) > > } > > > > intel_hdmi->set_infoframes(&encoder->base, false, NULL); > > + > > + intel_dp_dual_mode_set_tmds_output(intel_hdmi, false); > > } > > > > static void g4x_disable_hdmi(struct intel_encoder *encoder) > >
On 5/4/2016 5:19 PM, Ville Syrjälä wrote: > On Wed, May 04, 2016 at 03:43:30PM +0530, Sharma, Shashank wrote: >> >> On 5/3/2016 12:38 AM, ville.syrjala@linux.intel.com wrote: >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >>> >>> To save a bit of power, let's try to turn off the TMDS output buffers >>> in DP++ adaptors when we're not driving the port. >>> >>> v2: Let's not forget DDI, toss in a debug message while at it >>> v3: Just do the TMDS output control based on adaptor type. With the >>> helper getting passed the type, we wouldn't actually have to >>> check at all in the driver, but the check eliminates the debug >>> output more honest >>> >>> Cc: stable@vger.kernel.org >>> Cc: Tore Anderson <tore@fud.no> >>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >>> Cc: Shashank Sharma <shashank.sharma@intel.com> >>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_ddi.c | 12 ++++++++++++ >>> drivers/gpu/drm/i915/intel_drv.h | 1 + >>> drivers/gpu/drm/i915/intel_hdmi.c | 20 ++++++++++++++++++++ >>> 3 files changed, 33 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c >>> index 422ec81ef59b..a3600704e6d4 100644 >>> --- a/drivers/gpu/drm/i915/intel_ddi.c >>> +++ b/drivers/gpu/drm/i915/intel_ddi.c >>> @@ -1601,6 +1601,12 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) >>> enum port port = intel_ddi_get_encoder_port(intel_encoder); >>> int type = intel_encoder->type; >>> >>> + if (type == INTEL_OUTPUT_HDMI) { >> how to handle type2 active adapters here, which can show type = DP ? > > The dual mode concept shouldn't apply to active adapters. Those, as you > say, appear as DP and so will be handled as if they were native DP sinks > more or less. > > Or are you aware of active DP dongles that also implement the dual mode > standatd? Apart from LSPCON of course. Actually I'm not sure how LSPCON > will handle this. If we drive it in PCON mode, would it still respect > the dual mode register settings, including the TMDS OE# state? > I cross checked LSPCON specs, it respects TMDS_OE (0x80, 0x20) register only in LS mode (like stype 2 adapter), by putting it in low power state. So I guess you are right, we have to handle LSPCON separately, based on if it's in LS mode or PCON mode. May be while re-basing LSPCON layer on top of this new series, I can think of this. Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> Regards Shashank >>> + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); >>> + >>> + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); >>> + } >>> + >>> intel_prepare_ddi_buffer(intel_encoder); >>> >>> if (type == INTEL_OUTPUT_EDP) { >>> @@ -1667,6 +1673,12 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder) >>> DPLL_CTRL2_DDI_CLK_OFF(port))); >>> else if (INTEL_INFO(dev)->gen < 9) >>> I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE); >>> + >>> + if (type == INTEL_OUTPUT_HDMI) { >> Same as above. >> >> Regards >> Shashank >>> + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); >>> + >>> + intel_dp_dual_mode_set_tmds_output(intel_hdmi, false); >>> + } >>> } >>> >>> static void intel_enable_ddi(struct intel_encoder *intel_encoder) >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >>> index e94d18fb2ff1..dbcb80c70e2e 100644 >>> --- a/drivers/gpu/drm/i915/intel_drv.h >>> +++ b/drivers/gpu/drm/i915/intel_drv.h >>> @@ -1419,6 +1419,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, >>> struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder); >>> bool intel_hdmi_compute_config(struct intel_encoder *encoder, >>> struct intel_crtc_state *pipe_config); >>> +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable); >>> >>> >>> /* intel_lvds.c */ >>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c >>> index 31ca11134294..c4d93e6b4bed 100644 >>> --- a/drivers/gpu/drm/i915/intel_hdmi.c >>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c >>> @@ -836,6 +836,22 @@ static void hsw_set_infoframes(struct drm_encoder *encoder, >>> intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode); >>> } >>> >>> +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable) >>> +{ >>> + struct drm_i915_private *dev_priv = to_i915(intel_hdmi_to_dev(hdmi)); >>> + struct i2c_adapter *adapter = >>> + intel_gmbus_get_adapter(dev_priv, hdmi->ddc_bus); >>> + >>> + if (hdmi->dp_dual_mode.type < DRM_DP_DUAL_MODE_TYPE2_DVI) >>> + return; >>> + >>> + DRM_DEBUG_KMS("%s DP dual mode adaptor TMDS output\n", >>> + enable ? "Enabling" : "Disabling"); >>> + >>> + drm_dp_dual_mode_set_tmds_output(hdmi->dp_dual_mode.type, >>> + adapter, enable); >>> +} >>> + >>> static void intel_hdmi_prepare(struct intel_encoder *encoder) >>> { >>> struct drm_device *dev = encoder->base.dev; >>> @@ -845,6 +861,8 @@ static void intel_hdmi_prepare(struct intel_encoder *encoder) >>> const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode; >>> u32 hdmi_val; >>> >>> + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); >>> + >>> hdmi_val = SDVO_ENCODING_HDMI; >>> if (!HAS_PCH_SPLIT(dev) && crtc->config->limited_color_range) >>> hdmi_val |= HDMI_COLOR_RANGE_16_235; >>> @@ -1142,6 +1160,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder) >>> } >>> >>> intel_hdmi->set_infoframes(&encoder->base, false, NULL); >>> + >>> + intel_dp_dual_mode_set_tmds_output(intel_hdmi, false); >>> } >>> >>> static void g4x_disable_hdmi(struct intel_encoder *encoder) >>> >
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 422ec81ef59b..a3600704e6d4 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1601,6 +1601,12 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder) enum port port = intel_ddi_get_encoder_port(intel_encoder); int type = intel_encoder->type; + if (type == INTEL_OUTPUT_HDMI) { + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); + + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); + } + intel_prepare_ddi_buffer(intel_encoder); if (type == INTEL_OUTPUT_EDP) { @@ -1667,6 +1673,12 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder) DPLL_CTRL2_DDI_CLK_OFF(port))); else if (INTEL_INFO(dev)->gen < 9) I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE); + + if (type == INTEL_OUTPUT_HDMI) { + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); + + intel_dp_dual_mode_set_tmds_output(intel_hdmi, false); + } } static void intel_enable_ddi(struct intel_encoder *intel_encoder) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index e94d18fb2ff1..dbcb80c70e2e 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1419,6 +1419,7 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder); bool intel_hdmi_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config); +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable); /* intel_lvds.c */ diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 31ca11134294..c4d93e6b4bed 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -836,6 +836,22 @@ static void hsw_set_infoframes(struct drm_encoder *encoder, intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode); } +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable) +{ + struct drm_i915_private *dev_priv = to_i915(intel_hdmi_to_dev(hdmi)); + struct i2c_adapter *adapter = + intel_gmbus_get_adapter(dev_priv, hdmi->ddc_bus); + + if (hdmi->dp_dual_mode.type < DRM_DP_DUAL_MODE_TYPE2_DVI) + return; + + DRM_DEBUG_KMS("%s DP dual mode adaptor TMDS output\n", + enable ? "Enabling" : "Disabling"); + + drm_dp_dual_mode_set_tmds_output(hdmi->dp_dual_mode.type, + adapter, enable); +} + static void intel_hdmi_prepare(struct intel_encoder *encoder) { struct drm_device *dev = encoder->base.dev; @@ -845,6 +861,8 @@ static void intel_hdmi_prepare(struct intel_encoder *encoder) const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode; u32 hdmi_val; + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); + hdmi_val = SDVO_ENCODING_HDMI; if (!HAS_PCH_SPLIT(dev) && crtc->config->limited_color_range) hdmi_val |= HDMI_COLOR_RANGE_16_235; @@ -1142,6 +1160,8 @@ static void intel_disable_hdmi(struct intel_encoder *encoder) } intel_hdmi->set_infoframes(&encoder->base, false, NULL); + + intel_dp_dual_mode_set_tmds_output(intel_hdmi, false); } static void g4x_disable_hdmi(struct intel_encoder *encoder)