| Message ID | 1488271150-7423-6-git-send-email-shashank.sharma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 28, 2017 at 02:09:09PM +0530, Shashank Sharma wrote: > Geminilake platform sports a native HDMI 2.0 controller, and is > capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec > mendates scrambling for these higher clocks, for reduced RF footprint. > > This patch checks if the monitor supports scrambling, and if required, > enables it during the modeset. > > V2: Addressed review comments from Ville: > - Do not track scrambling status in DRM layer, track somewhere in > driver like in intel_crtc_state. > - Don't talk to monitor at such a low layer, set monitor scrambling > in intel_enable_ddi() before enabling the port. > > V3: Addressed review comments from Jani > - In comments, function names, use "sink" instead of "monitor", > so that the implementation could be close to the language of > HDMI spec. > > V4: Addressed review comment from Maarten > - scrambling -> hdmi_scrambling > high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio > > V5: Addressed review comments from Ville and Ander > - Do not modifiy the crtc_state after compute_config. Move all > scrambling and tmds_clock_ratio calcutations to compute_config. > - While setting scrambling for source/sink, do not check the > conditions again, just go by the crtc_state flags. This will > simplyfy the condition checks. > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 4 ++ > drivers/gpu/drm/i915/intel_ddi.c | 29 ++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 14 ++++++ > drivers/gpu/drm/i915/intel_hdmi.c | 98 +++++++++++++++++++++++++++++++++++++++ > 4 files changed, 145 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index ce6f096..1759b64 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7824,7 +7824,11 @@ enum { > #define TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12) > #define TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12) > #define TRANS_DDI_DP_VC_PAYLOAD_ALLOC (1<<8) > +#define TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7) > +#define TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6) > #define TRANS_DDI_BFI_ENABLE (1<<4) > +#define TRANS_DDI_HIGH_TMDS_CHAR_RATE (1<<4) > +#define TRANS_DDI_HDMI_SCRAMBLING (1<<0) > > /* DisplayPort Transport Control */ > #define _DP_TP_CTL_A 0x64040 > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index a7c08d7..24bc3a8 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1311,6 +1311,11 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc) > temp |= TRANS_DDI_MODE_SELECT_HDMI; > else > temp |= TRANS_DDI_MODE_SELECT_DVI; > + > + if (IS_GEMINILAKE(dev_priv)) > + temp = intel_hdmi_handle_source_scrambling( > + intel_encoder, > + intel_crtc->config, temp); > } else if (type == INTEL_OUTPUT_ANALOG) { > temp |= TRANS_DDI_MODE_SELECT_FDI; > temp |= (intel_crtc->config->fdi_lanes - 1) << 1; > @@ -1885,6 +1890,21 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder, > struct intel_digital_port *intel_dig_port = > enc_to_dig_port(encoder); > > + if (IS_GEMINILAKE(dev_priv)) { > + struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); > + /* > + * GLK sports a native HDMI 2.0 controller. If required > + * clock rate is > 340 Mhz && scrambling is supported > + * by sink, enable scrambling before enabling the > + * HDMI 2.0 port. The sink can choose to disable the > + * scrambling if it doesn't detect a scrambled within > + * 100 ms. > + */ > + intel_hdmi_handle_sink_scrambling(intel_encoder, > + conn_state->connector, > + crtc->config, true); > + } > + > /* In HDMI/DVI mode, the port width, and swing/emphasis values > * are ignored so nothing special needs to be done besides > * enabling the port. > @@ -1912,11 +1932,20 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder, > struct drm_connector_state *old_conn_state) > { > struct drm_encoder *encoder = &intel_encoder->base; > + struct drm_i915_private *dev_priv = encoder->dev->dev_private; > int type = intel_encoder->type; > > if (old_crtc_state->has_audio) > intel_audio_codec_disable(intel_encoder); > > + if ((type == INTEL_OUTPUT_HDMI) && IS_GEMINILAKE(dev_priv)) { ^ ^ Pointless parens. Also I'm nor sure we want the platform check here. Seems to me that we should be able to just explicitly configure the sink on every platform, iff the sink supports SCDC of course. > + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); > + > + intel_hdmi_handle_sink_scrambling(intel_encoder, > + old_conn_state->connector, > + intel_crtc->config, false); > + } > + > if (type == INTEL_OUTPUT_EDP) { > struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 907fb00..0e53fcd 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -695,6 +695,12 @@ struct intel_crtc_state { > > /* Gamma mode programmed on the pipe */ > uint32_t gamma_mode; > + > + /* HDMI scrambling status (sink) */ > + bool hdmi_scrambling; > + > + /* HDMI High TMDS char rate ratio (sink) */ > + bool hdmi_high_tmds_clock_ratio; > }; > > struct vlv_wm_state { > @@ -1626,6 +1632,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, > bool intel_hdmi_compute_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config, > struct drm_connector_state *conn_state); > +uint32_t > +intel_hdmi_handle_source_scrambling(struct intel_encoder *intel_encoder, > + struct intel_crtc_state *config, > + uint32_t hdmi_config); > +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder, > + struct drm_connector *connector, > + struct intel_crtc_state *config, > + bool enable); > void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable); > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index c2184f7..fdbeefd 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -34,6 +34,7 @@ > #include <drm/drm_atomic_helper.h> > #include <drm/drm_crtc.h> > #include <drm/drm_edid.h> > +#include <drm/drm_scdc_helper.h> > #include "intel_drv.h" > #include <drm/i915_drm.h> > #include <drm/intel_lpe_audio.h> > @@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, > struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; > + struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc; > int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock; > int clock_12bpc = clock_8bpc * 3 / 2; > int desired_bpp; > @@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, > > pipe_config->lane_count = 4; > > + if (scdc->scrambling.supported) { Here we should have a check whether the source support scrambling or not. > + if (scdc->scrambling.low_rates) > + pipe_config->hdmi_scrambling = true; > + > + if (pipe_config->port_clock > 340000) { > + pipe_config->hdmi_scrambling = true; > + pipe_config->hdmi_high_tmds_clock_ratio = true; > + } > + } > + > return true; > } > > @@ -1794,6 +1806,92 @@ static void intel_hdmi_destroy(struct drm_connector *connector) > intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE; > } > > +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder, > + struct drm_connector *connector, > + struct intel_crtc_state *config, > + bool enable) > +{ > + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base); > + struct drm_i915_private *dev_priv = connector->dev->dev_private; > + struct drm_scrambling *scrambling = > + &connector->display_info.hdmi.scdc.scrambling; > + struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv, > + intel_hdmi->ddc_bus); > + bool ret; > + > + if (!scrambling->supported) > + return; > + > + DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n", > + intel_encoder->base.name, connector->name); > + > + if (enable) { > + > + if (config->hdmi_high_tmds_clock_ratio) { > + /* Set TMDS bit clock ratio to 1/40 */ > + ret = drm_scdc_set_high_tmds_clock_ratio(adptr, true); > + if (!ret) { > + DRM_ERROR("Set high TMDS ratio failed\n"); > + return; > + } > + } > + > + if (config->hdmi_scrambling) { > + /* Enable sink scrambling */ > + ret = drm_scdc_set_scrambling(adptr, true); > + if (!ret) { > + DRM_ERROR("Can't enable sink scrambling\n"); > + return; > + } > + } > + > + DRM_DEBUG_KMS("sink scrambling enabled\n"); > + return; > + } > + > + if (config->hdmi_high_tmds_clock_ratio) { > + /* Set TMDS bit clock ratio back to 1/10 */ > + ret = !(drm_scdc_set_high_tmds_clock_ratio(adptr, false)); > + if (ret) > + DRM_ERROR("Reset high TMDS ratio failed\n"); > + } > + > + if (config->hdmi_scrambling) { > + /* Disable sink scrambling */ > + ret = !(drm_scdc_set_scrambling(adptr, false)); > + if (ret) > + DRM_ERROR("Disable sink scrambling failed\n"); > + } Seems a little complicated for what it's doing. I'd go for something simple like: foo(bool scrambling, bool clock_ratio) { set_scrambling(scrambling); set_clock_ratio(clock_ratio); } enable() { foo(crtc_state->hdmi_scrambling, crtc_state->clock_ratio); } disable() { foo(false, false); } > + > + DRM_DEBUG_KMS("sink scrambling disabled\n"); > +} > + > +uint32_t > +intel_hdmi_handle_source_scrambling(struct intel_encoder *intel_encoder, > + struct intel_crtc_state *config, uint32_t hdmi_config) > +{ > + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base); > + struct drm_connector *connector = &intel_hdmi->attached_connector->base; > + > + DRM_DEBUG_KMS("Setting scrambling for enc:%s connector:%s\n", > + intel_encoder->base.name, connector->name); > + > + hdmi_config &= ~(TRANS_DDI_HDMI_SCRAMBLING | > + TRANS_DDI_HIGH_TMDS_CHAR_RATE | > + TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ | > + TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE); > + > + if (config->hdmi_scrambling) > + hdmi_config |= (TRANS_DDI_HDMI_SCRAMBLING | > + TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ | > + TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE); > + > + if (config->hdmi_high_tmds_clock_ratio) > + hdmi_config |= TRANS_DDI_HIGH_TMDS_CHAR_RATE; IMO just inline these two things into the function that configures this register. Having to jump around between functions is just extra pain we don't need IMO. There should also be corresponding readout code for this in intel_ddi_get_config(), and matching PIPE_CONF_CHECK stuff in intel_pipe_config_compare(). > + > + return hdmi_config; > +} > + > static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv, > enum port port) > { > -- > 1.9.1
Regards Shashank On 3/1/2017 8:41 PM, Ville Syrjälä wrote: > On Tue, Feb 28, 2017 at 02:09:09PM +0530, Shashank Sharma wrote: >> Geminilake platform sports a native HDMI 2.0 controller, and is >> capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec >> mendates scrambling for these higher clocks, for reduced RF footprint. >> >> This patch checks if the monitor supports scrambling, and if required, >> enables it during the modeset. >> >> V2: Addressed review comments from Ville: >> - Do not track scrambling status in DRM layer, track somewhere in >> driver like in intel_crtc_state. >> - Don't talk to monitor at such a low layer, set monitor scrambling >> in intel_enable_ddi() before enabling the port. >> >> V3: Addressed review comments from Jani >> - In comments, function names, use "sink" instead of "monitor", >> so that the implementation could be close to the language of >> HDMI spec. >> >> V4: Addressed review comment from Maarten >> - scrambling -> hdmi_scrambling >> high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio >> >> V5: Addressed review comments from Ville and Ander >> - Do not modifiy the crtc_state after compute_config. Move all >> scrambling and tmds_clock_ratio calcutations to compute_config. >> - While setting scrambling for source/sink, do not check the >> conditions again, just go by the crtc_state flags. This will >> simplyfy the condition checks. >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 4 ++ >> drivers/gpu/drm/i915/intel_ddi.c | 29 ++++++++++++ >> drivers/gpu/drm/i915/intel_drv.h | 14 ++++++ >> drivers/gpu/drm/i915/intel_hdmi.c | 98 +++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 145 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h >> index ce6f096..1759b64 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -7824,7 +7824,11 @@ enum { >> #define TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12) >> #define TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12) >> #define TRANS_DDI_DP_VC_PAYLOAD_ALLOC (1<<8) >> +#define TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7) >> +#define TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6) >> #define TRANS_DDI_BFI_ENABLE (1<<4) >> +#define TRANS_DDI_HIGH_TMDS_CHAR_RATE (1<<4) >> +#define TRANS_DDI_HDMI_SCRAMBLING (1<<0) >> >> /* DisplayPort Transport Control */ >> #define _DP_TP_CTL_A 0x64040 >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c >> index a7c08d7..24bc3a8 100644 >> --- a/drivers/gpu/drm/i915/intel_ddi.c >> +++ b/drivers/gpu/drm/i915/intel_ddi.c >> @@ -1311,6 +1311,11 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc) >> temp |= TRANS_DDI_MODE_SELECT_HDMI; >> else >> temp |= TRANS_DDI_MODE_SELECT_DVI; >> + >> + if (IS_GEMINILAKE(dev_priv)) >> + temp = intel_hdmi_handle_source_scrambling( >> + intel_encoder, >> + intel_crtc->config, temp); >> } else if (type == INTEL_OUTPUT_ANALOG) { >> temp |= TRANS_DDI_MODE_SELECT_FDI; >> temp |= (intel_crtc->config->fdi_lanes - 1) << 1; >> @@ -1885,6 +1890,21 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder, >> struct intel_digital_port *intel_dig_port = >> enc_to_dig_port(encoder); >> >> + if (IS_GEMINILAKE(dev_priv)) { >> + struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); >> + /* >> + * GLK sports a native HDMI 2.0 controller. If required >> + * clock rate is > 340 Mhz && scrambling is supported >> + * by sink, enable scrambling before enabling the >> + * HDMI 2.0 port. The sink can choose to disable the >> + * scrambling if it doesn't detect a scrambled within >> + * 100 ms. >> + */ >> + intel_hdmi_handle_sink_scrambling(intel_encoder, >> + conn_state->connector, >> + crtc->config, true); >> + } >> + >> /* In HDMI/DVI mode, the port width, and swing/emphasis values >> * are ignored so nothing special needs to be done besides >> * enabling the port. >> @@ -1912,11 +1932,20 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder, >> struct drm_connector_state *old_conn_state) >> { >> struct drm_encoder *encoder = &intel_encoder->base; >> + struct drm_i915_private *dev_priv = encoder->dev->dev_private; >> int type = intel_encoder->type; >> >> if (old_crtc_state->has_audio) >> intel_audio_codec_disable(intel_encoder); >> >> + if ((type == INTEL_OUTPUT_HDMI) && IS_GEMINILAKE(dev_priv)) { > ^ ^ > > Pointless parens. Also I'm nor sure we want the platform check here. > Seems to me that we should be able to just explicitly configure the sink > on every platform, iff the sink supports SCDC of course. This is interesting, what if I connect a HDMI 2.0 sink, which supports scrambling to a GEN8 or non-lspcon GEN 9 device ? Shouldn't we check if the source also allows it ? >> + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); >> + >> + intel_hdmi_handle_sink_scrambling(intel_encoder, >> + old_conn_state->connector, >> + intel_crtc->config, false); >> + } >> + >> if (type == INTEL_OUTPUT_EDP) { >> struct intel_dp *intel_dp = enc_to_intel_dp(encoder); >> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index 907fb00..0e53fcd 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -695,6 +695,12 @@ struct intel_crtc_state { >> >> /* Gamma mode programmed on the pipe */ >> uint32_t gamma_mode; >> + >> + /* HDMI scrambling status (sink) */ >> + bool hdmi_scrambling; >> + >> + /* HDMI High TMDS char rate ratio (sink) */ >> + bool hdmi_high_tmds_clock_ratio; >> }; >> >> struct vlv_wm_state { >> @@ -1626,6 +1632,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, >> bool intel_hdmi_compute_config(struct intel_encoder *encoder, >> struct intel_crtc_state *pipe_config, >> struct drm_connector_state *conn_state); >> +uint32_t >> +intel_hdmi_handle_source_scrambling(struct intel_encoder *intel_encoder, >> + struct intel_crtc_state *config, >> + uint32_t hdmi_config); >> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder, >> + struct drm_connector *connector, >> + struct intel_crtc_state *config, >> + bool enable); >> void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable); >> >> >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c >> index c2184f7..fdbeefd 100644 >> --- a/drivers/gpu/drm/i915/intel_hdmi.c >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c >> @@ -34,6 +34,7 @@ >> #include <drm/drm_atomic_helper.h> >> #include <drm/drm_crtc.h> >> #include <drm/drm_edid.h> >> +#include <drm/drm_scdc_helper.h> >> #include "intel_drv.h" >> #include <drm/i915_drm.h> >> #include <drm/intel_lpe_audio.h> >> @@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, >> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); >> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); >> struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; >> + struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc; >> int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock; >> int clock_12bpc = clock_8bpc * 3 / 2; >> int desired_bpp; >> @@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, >> >> pipe_config->lane_count = 4; >> >> + if (scdc->scrambling.supported) { > Here we should have a check whether the source support scrambling or not. Ok, makes sense. > >> + if (scdc->scrambling.low_rates) >> + pipe_config->hdmi_scrambling = true; >> + >> + if (pipe_config->port_clock > 340000) { >> + pipe_config->hdmi_scrambling = true; >> + pipe_config->hdmi_high_tmds_clock_ratio = true; >> + } >> + } >> + >> return true; >> } >> >> @@ -1794,6 +1806,92 @@ static void intel_hdmi_destroy(struct drm_connector *connector) >> intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE; >> } >> >> +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder, >> + struct drm_connector *connector, >> + struct intel_crtc_state *config, >> + bool enable) >> +{ >> + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base); >> + struct drm_i915_private *dev_priv = connector->dev->dev_private; >> + struct drm_scrambling *scrambling = >> + &connector->display_info.hdmi.scdc.scrambling; >> + struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv, >> + intel_hdmi->ddc_bus); >> + bool ret; >> + >> + if (!scrambling->supported) >> + return; >> + >> + DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n", >> + intel_encoder->base.name, connector->name); >> + >> + if (enable) { >> + >> + if (config->hdmi_high_tmds_clock_ratio) { >> + /* Set TMDS bit clock ratio to 1/40 */ >> + ret = drm_scdc_set_high_tmds_clock_ratio(adptr, true); >> + if (!ret) { >> + DRM_ERROR("Set high TMDS ratio failed\n"); >> + return; >> + } >> + } >> + >> + if (config->hdmi_scrambling) { >> + /* Enable sink scrambling */ >> + ret = drm_scdc_set_scrambling(adptr, true); >> + if (!ret) { >> + DRM_ERROR("Can't enable sink scrambling\n"); >> + return; >> + } >> + } >> + >> + DRM_DEBUG_KMS("sink scrambling enabled\n"); >> + return; >> + } >> + >> + if (config->hdmi_high_tmds_clock_ratio) { >> + /* Set TMDS bit clock ratio back to 1/10 */ >> + ret = !(drm_scdc_set_high_tmds_clock_ratio(adptr, false)); >> + if (ret) >> + DRM_ERROR("Reset high TMDS ratio failed\n"); >> + } >> + >> + if (config->hdmi_scrambling) { >> + /* Disable sink scrambling */ >> + ret = !(drm_scdc_set_scrambling(adptr, false)); >> + if (ret) >> + DRM_ERROR("Disable sink scrambling failed\n"); >> + } > Seems a little complicated for what it's doing. > > I'd go for something simple like: > > foo(bool scrambling, bool clock_ratio) > { > set_scrambling(scrambling); > set_clock_ratio(clock_ratio); > } > > enable() > { > foo(crtc_state->hdmi_scrambling, crtc_state->clock_ratio); > } > > disable() > { > foo(false, false); > } In the first look, I thought intel_hdmi_handle_sink_scrambling is your foo, but later I realized, you dont want an if (enable) cond.... will take care of it. > >> + >> + DRM_DEBUG_KMS("sink scrambling disabled\n"); >> +} >> + >> +uint32_t >> +intel_hdmi_handle_source_scrambling(struct intel_encoder *intel_encoder, >> + struct intel_crtc_state *config, uint32_t hdmi_config) >> +{ >> + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base); >> + struct drm_connector *connector = &intel_hdmi->attached_connector->base; >> + >> + DRM_DEBUG_KMS("Setting scrambling for enc:%s connector:%s\n", >> + intel_encoder->base.name, connector->name); >> + >> + hdmi_config &= ~(TRANS_DDI_HDMI_SCRAMBLING | >> + TRANS_DDI_HIGH_TMDS_CHAR_RATE | >> + TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ | >> + TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE); >> + >> + if (config->hdmi_scrambling) >> + hdmi_config |= (TRANS_DDI_HDMI_SCRAMBLING | >> + TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ | >> + TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE); >> + >> + if (config->hdmi_high_tmds_clock_ratio) >> + hdmi_config |= TRANS_DDI_HIGH_TMDS_CHAR_RATE; > IMO just inline these two things into the function that configures > this register. Having to jump around between functions is just > extra pain we don't need IMO. Not really, this function is also making sure, that the scrambling bits are cleared, and only enabled which is required. Also, this is making code more readable, segregating scrambling functionalities, and better for debugging too. Lets keep this one if you please allow. > There should also be corresponding readout code for this in > intel_ddi_get_config(), and matching PIPE_CONF_CHECK stuff in > intel_pipe_config_compare(). Agree, will add that too. - Shashank >> + >> + return hdmi_config; >> +} >> + >> static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv, >> enum port port) >> { >> -- >> 1.9.1
On Thu, 2017-03-02 at 08:25 +0530, Sharma, Shashank wrote: > Regards > > Shashank > > > On 3/1/2017 8:41 PM, Ville Syrjälä wrote: > > On Tue, Feb 28, 2017 at 02:09:09PM +0530, Shashank Sharma wrote: > > > Geminilake platform sports a native HDMI 2.0 controller, and is > > > capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec > > > mendates scrambling for these higher clocks, for reduced RF footprint. > > > > > > This patch checks if the monitor supports scrambling, and if required, > > > enables it during the modeset. > > > > > > V2: Addressed review comments from Ville: > > > - Do not track scrambling status in DRM layer, track somewhere in > > > driver like in intel_crtc_state. > > > - Don't talk to monitor at such a low layer, set monitor scrambling > > > in intel_enable_ddi() before enabling the port. > > > > > > V3: Addressed review comments from Jani > > > - In comments, function names, use "sink" instead of "monitor", > > > so that the implementation could be close to the language of > > > HDMI spec. > > > > > > V4: Addressed review comment from Maarten > > > - scrambling -> hdmi_scrambling > > > high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio > > > > > > V5: Addressed review comments from Ville and Ander > > > - Do not modifiy the crtc_state after compute_config. Move all > > > scrambling and tmds_clock_ratio calcutations to compute_config. > > > - While setting scrambling for source/sink, do not check the > > > conditions again, just go by the crtc_state flags. This will > > > simplyfy the condition checks. > > > > > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > > > --- > > > drivers/gpu/drm/i915/i915_reg.h | 4 ++ > > > drivers/gpu/drm/i915/intel_ddi.c | 29 ++++++++++++ > > > drivers/gpu/drm/i915/intel_drv.h | 14 ++++++ > > > drivers/gpu/drm/i915/intel_hdmi.c | 98 +++++++++++++++++++++++++++++++++++++++ > > > 4 files changed, 145 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > > index ce6f096..1759b64 100644 > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > @@ -7824,7 +7824,11 @@ enum { > > > #define TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12) > > > #define TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12) > > > #define TRANS_DDI_DP_VC_PAYLOAD_ALLOC (1<<8) > > > +#define TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7) > > > +#define TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6) > > > #define TRANS_DDI_BFI_ENABLE (1<<4) > > > +#define TRANS_DDI_HIGH_TMDS_CHAR_RATE (1<<4) > > > +#define TRANS_DDI_HDMI_SCRAMBLING (1<<0) > > > > > > /* DisplayPort Transport Control */ > > > #define _DP_TP_CTL_A 0x64040 > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > > > index a7c08d7..24bc3a8 100644 > > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > > @@ -1311,6 +1311,11 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc) > > > temp |= TRANS_DDI_MODE_SELECT_HDMI; > > > else > > > temp |= TRANS_DDI_MODE_SELECT_DVI; > > > + > > > + if (IS_GEMINILAKE(dev_priv)) > > > + temp = intel_hdmi_handle_source_scrambling( > > > + intel_encoder, > > > + intel_crtc->config, temp); > > > } else if (type == INTEL_OUTPUT_ANALOG) { > > > temp |= TRANS_DDI_MODE_SELECT_FDI; > > > temp |= (intel_crtc->config->fdi_lanes - 1) << 1; > > > @@ -1885,6 +1890,21 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder, > > > struct intel_digital_port *intel_dig_port = > > > enc_to_dig_port(encoder); > > > > > > + if (IS_GEMINILAKE(dev_priv)) { > > > + struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); > > > + /* > > > + * GLK sports a native HDMI 2.0 controller. If required > > > + * clock rate is > 340 Mhz && scrambling is supported > > > + * by sink, enable scrambling before enabling the > > > + * HDMI 2.0 port. The sink can choose to disable the > > > + * scrambling if it doesn't detect a scrambled within > > > + * 100 ms. > > > + */ > > > + intel_hdmi_handle_sink_scrambling(intel_encoder, > > > + conn_state->connector, > > > + crtc->config, true); > > > + } > > > + > > > /* In HDMI/DVI mode, the port width, and swing/emphasis values > > > * are ignored so nothing special needs to be done besides > > > * enabling the port. > > > @@ -1912,11 +1932,20 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder, > > > struct drm_connector_state *old_conn_state) > > > { > > > struct drm_encoder *encoder = &intel_encoder->base; > > > + struct drm_i915_private *dev_priv = encoder->dev->dev_private; > > > int type = intel_encoder->type; > > > > > > if (old_crtc_state->has_audio) > > > intel_audio_codec_disable(intel_encoder); > > > > > > + if ((type == INTEL_OUTPUT_HDMI) && IS_GEMINILAKE(dev_priv)) { > > > > ^ ^ > > > > Pointless parens. Also I'm nor sure we want the platform check here. > > Seems to me that we should be able to just explicitly configure the sink > > on every platform, iff the sink supports SCDC of course. > > This is interesting, what if I connect a HDMI 2.0 sink, which supports > scrambling to a GEN8 or non-lspcon > GEN 9 device ? Shouldn't we check if the source also allows it ? > > > + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); > > > + > > > + intel_hdmi_handle_sink_scrambling(intel_encoder, > > > + old_conn_state->connector, > > > + intel_crtc->config, false); > > > + } > > > + > > > if (type == INTEL_OUTPUT_EDP) { > > > struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > > > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > > index 907fb00..0e53fcd 100644 > > > --- a/drivers/gpu/drm/i915/intel_drv.h > > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > > @@ -695,6 +695,12 @@ struct intel_crtc_state { > > > > > > /* Gamma mode programmed on the pipe */ > > > uint32_t gamma_mode; > > > + > > > + /* HDMI scrambling status (sink) */ > > > + bool hdmi_scrambling; > > > + > > > + /* HDMI High TMDS char rate ratio (sink) */ > > > + bool hdmi_high_tmds_clock_ratio; > > > }; > > > > > > struct vlv_wm_state { > > > @@ -1626,6 +1632,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, > > > bool intel_hdmi_compute_config(struct intel_encoder *encoder, > > > struct intel_crtc_state *pipe_config, > > > struct drm_connector_state *conn_state); > > > +uint32_t > > > +intel_hdmi_handle_source_scrambling(struct intel_encoder *intel_encoder, > > > + struct intel_crtc_state *config, > > > + uint32_t hdmi_config); > > > +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder, > > > + struct drm_connector *connector, > > > + struct intel_crtc_state *config, > > > + bool enable); > > > void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable); > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > > > index c2184f7..fdbeefd 100644 > > > --- a/drivers/gpu/drm/i915/intel_hdmi.c > > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > > > @@ -34,6 +34,7 @@ > > > #include <drm/drm_atomic_helper.h> > > > #include <drm/drm_crtc.h> > > > #include <drm/drm_edid.h> > > > +#include <drm/drm_scdc_helper.h> > > > #include "intel_drv.h" > > > #include <drm/i915_drm.h> > > > #include <drm/intel_lpe_audio.h> > > > @@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, > > > struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); > > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > > struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; > > > + struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc; > > > int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock; > > > int clock_12bpc = clock_8bpc * 3 / 2; > > > int desired_bpp; > > > @@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, > > > > > > pipe_config->lane_count = 4; > > > > > > + if (scdc->scrambling.supported) { > > > > Here we should have a check whether the source support scrambling or not. > > Ok, makes sense. > > > > > + if (scdc->scrambling.low_rates) > > > + pipe_config->hdmi_scrambling = true; > > > + > > > + if (pipe_config->port_clock > 340000) { > > > + pipe_config->hdmi_scrambling = true; > > > + pipe_config->hdmi_high_tmds_clock_ratio = true; > > > + } > > > + } > > > + > > > return true; > > > } > > > > > > @@ -1794,6 +1806,92 @@ static void intel_hdmi_destroy(struct drm_connector *connector) > > > intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE; > > > } > > > > > > +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder, > > > + struct drm_connector *connector, > > > + struct intel_crtc_state *config, > > > + bool enable) > > > +{ > > > + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base); > > > + struct drm_i915_private *dev_priv = connector->dev->dev_private; > > > + struct drm_scrambling *scrambling = > > > + &connector->display_info.hdmi.scdc.scrambling; > > > + struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv, > > > + intel_hdmi->ddc_bus); > > > + bool ret; > > > + > > > + if (!scrambling->supported) > > > + return; > > > + > > > + DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n", > > > + intel_encoder->base.name, connector->name); > > > + > > > + if (enable) { > > > + > > > + if (config->hdmi_high_tmds_clock_ratio) { > > > + /* Set TMDS bit clock ratio to 1/40 */ > > > + ret = drm_scdc_set_high_tmds_clock_ratio(adptr, true); > > > + if (!ret) { > > > + DRM_ERROR("Set high TMDS ratio failed\n"); > > > + return; > > > + } > > > + } > > > + > > > + if (config->hdmi_scrambling) { > > > + /* Enable sink scrambling */ > > > + ret = drm_scdc_set_scrambling(adptr, true); > > > + if (!ret) { > > > + DRM_ERROR("Can't enable sink scrambling\n"); > > > + return; > > > + } > > > + } > > > + > > > + DRM_DEBUG_KMS("sink scrambling enabled\n"); > > > + return; > > > + } > > > + > > > + if (config->hdmi_high_tmds_clock_ratio) { > > > + /* Set TMDS bit clock ratio back to 1/10 */ > > > + ret = !(drm_scdc_set_high_tmds_clock_ratio(adptr, false)); > > > + if (ret) > > > + DRM_ERROR("Reset high TMDS ratio failed\n"); > > > + } > > > + > > > + if (config->hdmi_scrambling) { > > > + /* Disable sink scrambling */ > > > + ret = !(drm_scdc_set_scrambling(adptr, false)); > > > + if (ret) > > > + DRM_ERROR("Disable sink scrambling failed\n"); > > > + } > > > > Seems a little complicated for what it's doing. > > > > I'd go for something simple like: > > > > foo(bool scrambling, bool clock_ratio) > > { > > set_scrambling(scrambling); > > set_clock_ratio(clock_ratio); > > } > > > > enable() > > { > > foo(crtc_state->hdmi_scrambling, crtc_state->clock_ratio); > > } > > > > disable() > > { > > foo(false, false); > > } > > In the first look, I thought intel_hdmi_handle_sink_scrambling is your > foo, but later I realized, > you dont want an if (enable) cond.... will take care of it. > > > > > + > > > + DRM_DEBUG_KMS("sink scrambling disabled\n"); > > > +} > > > + > > > +uint32_t > > > +intel_hdmi_handle_source_scrambling(struct intel_encoder *intel_encoder, > > > + struct intel_crtc_state *config, uint32_t hdmi_config) > > > +{ > > > + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base); > > > + struct drm_connector *connector = &intel_hdmi->attached_connector->base; > > > + > > > + DRM_DEBUG_KMS("Setting scrambling for enc:%s connector:%s\n", > > > + intel_encoder->base.name, connector->name); > > > + > > > + hdmi_config &= ~(TRANS_DDI_HDMI_SCRAMBLING | > > > + TRANS_DDI_HIGH_TMDS_CHAR_RATE | > > > + TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ | > > > + TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE); > > > + > > > + if (config->hdmi_scrambling) > > > + hdmi_config |= (TRANS_DDI_HDMI_SCRAMBLING | > > > + TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ | > > > + TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE); > > > + > > > + if (config->hdmi_high_tmds_clock_ratio) > > > + hdmi_config |= TRANS_DDI_HIGH_TMDS_CHAR_RATE; > > > > IMO just inline these two things into the function that configures > > this register. Having to jump around between functions is just > > extra pain we don't need IMO. > > Not really, this function is also making sure, that the scrambling bits > are cleared, There is no read-modify-write in intel_ddi_enable_transcoder_func(). Every bit that is not explicitly set is cleared. > and only > enabled which is required. Also, this is making code more readable, > segregating scrambling > functionalities, and better for debugging too. Lets keep this one if you > please allow. I think the extra function makes it harder to read. The function intel_ddi_enable_transcoder_func() only translates the current configuration to the right bits that need to be programmed to TRANS_DDI_FUNC_CTL. That belongs in intel_ddi.c. To help with debugging, add HDMI scrambling prints to intel_dump_pipe_config(). That's actually more useful than the print in this function since it can also tell us if scrambling was enabled for a configuration read out from hw state. Ander > > There should also be corresponding readout code for this in > > intel_ddi_get_config(), and matching PIPE_CONF_CHECK stuff in > > intel_pipe_config_compare(). > > Agree, will add that too. > > - Shashank > > > + > > > + return hdmi_config; > > > +} > > > + > > > static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv, > > > enum port port) > > > { > > > -- > > > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index ce6f096..1759b64 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7824,7 +7824,11 @@ enum { #define TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12) #define TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12) #define TRANS_DDI_DP_VC_PAYLOAD_ALLOC (1<<8) +#define TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7) +#define TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6) #define TRANS_DDI_BFI_ENABLE (1<<4) +#define TRANS_DDI_HIGH_TMDS_CHAR_RATE (1<<4) +#define TRANS_DDI_HDMI_SCRAMBLING (1<<0) /* DisplayPort Transport Control */ #define _DP_TP_CTL_A 0x64040 diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index a7c08d7..24bc3a8 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1311,6 +1311,11 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc) temp |= TRANS_DDI_MODE_SELECT_HDMI; else temp |= TRANS_DDI_MODE_SELECT_DVI; + + if (IS_GEMINILAKE(dev_priv)) + temp = intel_hdmi_handle_source_scrambling( + intel_encoder, + intel_crtc->config, temp); } else if (type == INTEL_OUTPUT_ANALOG) { temp |= TRANS_DDI_MODE_SELECT_FDI; temp |= (intel_crtc->config->fdi_lanes - 1) << 1; @@ -1885,6 +1890,21 @@ static void intel_enable_ddi(struct intel_encoder *intel_encoder, struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder); + if (IS_GEMINILAKE(dev_priv)) { + struct intel_crtc *crtc = to_intel_crtc(encoder->crtc); + /* + * GLK sports a native HDMI 2.0 controller. If required + * clock rate is > 340 Mhz && scrambling is supported + * by sink, enable scrambling before enabling the + * HDMI 2.0 port. The sink can choose to disable the + * scrambling if it doesn't detect a scrambled within + * 100 ms. + */ + intel_hdmi_handle_sink_scrambling(intel_encoder, + conn_state->connector, + crtc->config, true); + } + /* In HDMI/DVI mode, the port width, and swing/emphasis values * are ignored so nothing special needs to be done besides * enabling the port. @@ -1912,11 +1932,20 @@ static void intel_disable_ddi(struct intel_encoder *intel_encoder, struct drm_connector_state *old_conn_state) { struct drm_encoder *encoder = &intel_encoder->base; + struct drm_i915_private *dev_priv = encoder->dev->dev_private; int type = intel_encoder->type; if (old_crtc_state->has_audio) intel_audio_codec_disable(intel_encoder); + if ((type == INTEL_OUTPUT_HDMI) && IS_GEMINILAKE(dev_priv)) { + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc); + + intel_hdmi_handle_sink_scrambling(intel_encoder, + old_conn_state->connector, + intel_crtc->config, false); + } + if (type == INTEL_OUTPUT_EDP) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 907fb00..0e53fcd 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -695,6 +695,12 @@ struct intel_crtc_state { /* Gamma mode programmed on the pipe */ uint32_t gamma_mode; + + /* HDMI scrambling status (sink) */ + bool hdmi_scrambling; + + /* HDMI High TMDS char rate ratio (sink) */ + bool hdmi_high_tmds_clock_ratio; }; struct vlv_wm_state { @@ -1626,6 +1632,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port, bool intel_hdmi_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state); +uint32_t +intel_hdmi_handle_source_scrambling(struct intel_encoder *intel_encoder, + struct intel_crtc_state *config, + uint32_t hdmi_config); +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder, + struct drm_connector *connector, + struct intel_crtc_state *config, + bool enable); void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable); diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index c2184f7..fdbeefd 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -34,6 +34,7 @@ #include <drm/drm_atomic_helper.h> #include <drm/drm_crtc.h> #include <drm/drm_edid.h> +#include <drm/drm_scdc_helper.h> #include "intel_drv.h" #include <drm/i915_drm.h> #include <drm/intel_lpe_audio.h> @@ -1316,6 +1317,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; + struct drm_scdc *scdc = &conn_state->connector->display_info.hdmi.scdc; int clock_8bpc = pipe_config->base.adjusted_mode.crtc_clock; int clock_12bpc = clock_8bpc * 3 / 2; int desired_bpp; @@ -1385,6 +1387,16 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder, pipe_config->lane_count = 4; + if (scdc->scrambling.supported) { + if (scdc->scrambling.low_rates) + pipe_config->hdmi_scrambling = true; + + if (pipe_config->port_clock > 340000) { + pipe_config->hdmi_scrambling = true; + pipe_config->hdmi_high_tmds_clock_ratio = true; + } + } + return true; } @@ -1794,6 +1806,92 @@ static void intel_hdmi_destroy(struct drm_connector *connector) intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE; } +void intel_hdmi_handle_sink_scrambling(struct intel_encoder *intel_encoder, + struct drm_connector *connector, + struct intel_crtc_state *config, + bool enable) +{ + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base); + struct drm_i915_private *dev_priv = connector->dev->dev_private; + struct drm_scrambling *scrambling = + &connector->display_info.hdmi.scdc.scrambling; + struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv, + intel_hdmi->ddc_bus); + bool ret; + + if (!scrambling->supported) + return; + + DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n", + intel_encoder->base.name, connector->name); + + if (enable) { + + if (config->hdmi_high_tmds_clock_ratio) { + /* Set TMDS bit clock ratio to 1/40 */ + ret = drm_scdc_set_high_tmds_clock_ratio(adptr, true); + if (!ret) { + DRM_ERROR("Set high TMDS ratio failed\n"); + return; + } + } + + if (config->hdmi_scrambling) { + /* Enable sink scrambling */ + ret = drm_scdc_set_scrambling(adptr, true); + if (!ret) { + DRM_ERROR("Can't enable sink scrambling\n"); + return; + } + } + + DRM_DEBUG_KMS("sink scrambling enabled\n"); + return; + } + + if (config->hdmi_high_tmds_clock_ratio) { + /* Set TMDS bit clock ratio back to 1/10 */ + ret = !(drm_scdc_set_high_tmds_clock_ratio(adptr, false)); + if (ret) + DRM_ERROR("Reset high TMDS ratio failed\n"); + } + + if (config->hdmi_scrambling) { + /* Disable sink scrambling */ + ret = !(drm_scdc_set_scrambling(adptr, false)); + if (ret) + DRM_ERROR("Disable sink scrambling failed\n"); + } + + DRM_DEBUG_KMS("sink scrambling disabled\n"); +} + +uint32_t +intel_hdmi_handle_source_scrambling(struct intel_encoder *intel_encoder, + struct intel_crtc_state *config, uint32_t hdmi_config) +{ + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base); + struct drm_connector *connector = &intel_hdmi->attached_connector->base; + + DRM_DEBUG_KMS("Setting scrambling for enc:%s connector:%s\n", + intel_encoder->base.name, connector->name); + + hdmi_config &= ~(TRANS_DDI_HDMI_SCRAMBLING | + TRANS_DDI_HIGH_TMDS_CHAR_RATE | + TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ | + TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE); + + if (config->hdmi_scrambling) + hdmi_config |= (TRANS_DDI_HDMI_SCRAMBLING | + TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ | + TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE); + + if (config->hdmi_high_tmds_clock_ratio) + hdmi_config |= TRANS_DDI_HIGH_TMDS_CHAR_RATE; + + return hdmi_config; +} + static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv, enum port port) {
Geminilake platform sports a native HDMI 2.0 controller, and is capable of driving pixel-clocks upto 594Mhz. HDMI 2.0 spec mendates scrambling for these higher clocks, for reduced RF footprint. This patch checks if the monitor supports scrambling, and if required, enables it during the modeset. V2: Addressed review comments from Ville: - Do not track scrambling status in DRM layer, track somewhere in driver like in intel_crtc_state. - Don't talk to monitor at such a low layer, set monitor scrambling in intel_enable_ddi() before enabling the port. V3: Addressed review comments from Jani - In comments, function names, use "sink" instead of "monitor", so that the implementation could be close to the language of HDMI spec. V4: Addressed review comment from Maarten - scrambling -> hdmi_scrambling high_tmds_clock_ratio -> hdmi_high_tmds_clock_ratio V5: Addressed review comments from Ville and Ander - Do not modifiy the crtc_state after compute_config. Move all scrambling and tmds_clock_ratio calcutations to compute_config. - While setting scrambling for source/sink, do not check the conditions again, just go by the crtc_state flags. This will simplyfy the condition checks. Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 4 ++ drivers/gpu/drm/i915/intel_ddi.c | 29 ++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 14 ++++++ drivers/gpu/drm/i915/intel_hdmi.c | 98 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+)