From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Sharma, Shashank" Subject: Re: [PATCH v3 3/4] drm/i915: Enable/disable TMDS output buffers in DP++ adaptor as needed Date: Wed, 04 May 2016 19:18:11 +0530 Message-ID: <5729FD9B.5020309@intel.com> References: <1462216105-20881-1-git-send-email-ville.syrjala@linux.intel.com> <1462216105-20881-4-git-send-email-ville.syrjala@linux.intel.com> <5729CB4A.90607@intel.com> <20160504114959.GP4329@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20160504114959.GP4329@intel.com> Sender: stable-owner@vger.kernel.org To: =?windows-1252?Q?Ville_Syrj=E4l=E4?= Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, stable@vger.kernel.org, Tore Anderson , Paulo Zanoni , Daniel Vetter List-Id: intel-gfx@lists.freedesktop.org On 5/4/2016 5:19 PM, Ville Syrj=E4l=E4 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=E4l=E4 >>> >>> To save a bit of power, let's try to turn off the TMDS output buffe= rs >>> 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 debu= g >>> output more honest >>> >>> Cc: stable@vger.kernel.org >>> Cc: Tore Anderson >>> Cc: Paulo Zanoni >>> Cc: Shashank Sharma >>> Cc: Daniel Vetter >>> Signed-off-by: Ville Syrj=E4l=E4 >>> --- >>> 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/i91= 5/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 inte= l_encoder *intel_encoder) >>> enum port port =3D intel_ddi_get_encoder_port(intel_encoder); >>> int type =3D intel_encoder->type; >>> >>> + if (type =3D=3D INTEL_OUTPUT_HDMI) { >> how to handle type2 active adapters here, which can show type =3D DP= ? > > The dual mode concept shouldn't apply to active adapters. Those, as y= ou > say, appear as DP and so will be handled as if they were native DP si= nks > more or less. > > Or are you aware of active DP dongles that also implement the dual mo= de > standatd? Apart from LSPCON of course. Actually I'm not sure how LSPC= ON > 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= =20 only in LS mode (like stype 2 adapter), by putting it in low power=20 state. So I guess you are right, we have to handle LSPCON separately,=20 based on if it's in LS mode or PCON mode. May be while re-basing LSPCO= N=20 layer on top of this new series, I can think of this. Reviewed-by: Shashank Sharma Regards Shashank >>> + struct intel_hdmi *intel_hdmi =3D enc_to_intel_hdmi(encoder); >>> + >>> + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); >>> + } >>> + >>> intel_prepare_ddi_buffer(intel_encoder); >>> >>> if (type =3D=3D INTEL_OUTPUT_EDP) { >>> @@ -1667,6 +1673,12 @@ static void intel_ddi_post_disable(struct in= tel_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 =3D=3D INTEL_OUTPUT_HDMI) { >> Same as above. >> >> Regards >> Shashank >>> + struct intel_hdmi *intel_hdmi =3D 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/i91= 5/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_d= igital_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, b= ool enable); >>> >>> >>> /* intel_lvds.c */ >>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i9= 15/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_enco= der *encoder, >>> intel_hdmi_set_hdmi_infoframe(encoder, adjusted_mode); >>> } >>> >>> +void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, b= ool enable) >>> +{ >>> + struct drm_i915_private *dev_priv =3D to_i915(intel_hdmi_to_dev(h= dmi)); >>> + struct i2c_adapter *adapter =3D >>> + 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 =3D encoder->base.dev; >>> @@ -845,6 +861,8 @@ static void intel_hdmi_prepare(struct intel_enc= oder *encoder) >>> const struct drm_display_mode *adjusted_mode =3D &crtc->config-= >base.adjusted_mode; >>> u32 hdmi_val; >>> >>> + intel_dp_dual_mode_set_tmds_output(intel_hdmi, true); >>> + >>> hdmi_val =3D SDVO_ENCODING_HDMI; >>> if (!HAS_PCH_SPLIT(dev) && crtc->config->limited_color_range) >>> hdmi_val |=3D HDMI_COLOR_RANGE_16_235; >>> @@ -1142,6 +1160,8 @@ static void intel_disable_hdmi(struct intel_e= ncoder *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) >>> >