On Wed, Mar 08, 2017 at 07:26:45PM +0200, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 3/8/2017 7:17 PM, Ville Syrjälä wrote:
> > On Wed, Mar 08, 2017 at 07:07:48PM +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.
> >>
> >> V6: Addressed review comments from Ville
> >> - Do not add IS_GLK check in disable/enable function, instead add it
> >> in compute_config, while setting state flags.
> >> - Remove unnecessary paranthesis.
> >> - Simplyfy handle_sink_scrambling function as suggested.
> >> - Add readout code for scrambling status in get_ddi_config and add a
> >> check for the same in pipe_config_compare.
> >>
> >> V7: Addressed review comments from Ander/Ville
> >> - No separate function for source scrambling, make it inline
> >> - Align the last line of the macro TRANS_DDI_HDMI_SCRAMBLING_MASK
> >> - Do not add platform check while setting source scrambling
> >> - Use pipe_config instead of crtc->config to set sink scrambling
> >> - To readout scrambling status, Compare with SCRAMBLING_MASK
> >> not any of its bits
> >> - Remove platform check in intel_pipe_config_compare while checking
> >> scrambling status
> >>
> >> V8: Fixed mege conflict, Addressed review comments from Ander
> >> - Remove the desciption/comment about scrambling fom the caller, move
> >> it to the function
> >> - Move the IS_GLK check into scrambling function
> >> - Fix alignment
> >>
> >> Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_reg.h | 7 ++++
> >> drivers/gpu/drm/i915/intel_ddi.c | 19 +++++++++++
> >> drivers/gpu/drm/i915/intel_display.c | 3 ++
> >> drivers/gpu/drm/i915/intel_drv.h | 10 ++++++
> >> drivers/gpu/drm/i915/intel_hdmi.c | 62
> >> ++++++++++++++++++++++++++++++++++++
> >> 5 files changed, 101 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> b/drivers/gpu/drm/i915/i915_reg.h
> >> index cc843f9..2d50fdc 100644
> >> --- a/drivers/gpu/drm/i915/i915_reg.h
> >> +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> @@ -7830,7 +7830,14 @@ 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)
> >> +#define TRANS_DDI_HDMI_SCRAMBLING_MASK
> >> (TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE \
> >> + | TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ \
> >> + | TRANS_DDI_HDMI_SCRAMBLING)
> >>
> >> /* 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 0467676..b3e4c4a 100644
> >> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> @@ -1309,6 +1309,11 @@ void intel_ddi_enable_transcoder_func(const struct
> >> intel_crtc_state *crtc_state)
> >> temp |= TRANS_DDI_MODE_SELECT_HDMI;
> >> else
> >> temp |= TRANS_DDI_MODE_SELECT_DVI;
> >> +
> >> + if (crtc_state->hdmi_scrambling)
> >> + temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
> >> + if (crtc_state->hdmi_high_tmds_clock_ratio)
> >> + temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
> >> } else if (type == INTEL_OUTPUT_ANALOG) {
> >> temp |= TRANS_DDI_MODE_SELECT_FDI;
> >> temp |= (crtc_state->fdi_lanes - 1) << 1;
> >> @@ -1882,6 +1887,9 @@ static void intel_enable_ddi(struct intel_encoder
> >> *intel_encoder,
> >> struct intel_digital_port *intel_dig_port =
> >> enc_to_dig_port(encoder);
> >>
> >> + intel_hdmi_handle_sink_scrambling(intel_encoder,
> >> + conn_state->connector, pipe_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.
> >> @@ -1914,6 +1922,11 @@ static void intel_disable_ddi(struct intel_encoder
> >> *intel_encoder,
> >> if (old_crtc_state->has_audio)
> >> intel_audio_codec_disable(intel_encoder);
> >>
> >> + if (type == INTEL_OUTPUT_HDMI) {
> >> + intel_hdmi_handle_sink_scrambling(intel_encoder,
> >> + old_conn_state->connector, old_crtc_state, false);
> >> + }
> >> +
> >> if (type == INTEL_OUTPUT_EDP) {
> >> struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> >>
> >> @@ -2040,6 +2053,12 @@ void intel_ddi_get_config(struct intel_encoder
> >> *encoder,
> >>
> >> if (intel_hdmi->infoframe_enabled(&encoder->base, pipe_config))
> >> pipe_config->has_infoframe = true;
> >> +
> >> + if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
> >> + TRANS_DDI_HDMI_SCRAMBLING_MASK)
> >> + pipe_config->hdmi_scrambling = true;
> >> + if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
> >> + pipe_config->hdmi_high_tmds_clock_ratio = true;
> >> /* fall through */
> >> case TRANS_DDI_MODE_SELECT_DVI:
> >> pipe_config->lane_count = 4;
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >> b/drivers/gpu/drm/i915/intel_display.c
> >> index e77ca7d..fb5e670 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -11701,6 +11701,9 @@ static void __printf(3, 4)
> >> if ((INTEL_GEN(dev_priv) < 8 && !IS_HASWELL(dev_priv)) ||
> >> IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> >> PIPE_CONF_CHECK_I(limited_color_range);
> >> +
> >> + PIPE_CONF_CHECK_I(hdmi_scrambling);
> >> + PIPE_CONF_CHECK_I(hdmi_high_tmds_clock_ratio);
> >> PIPE_CONF_CHECK_I(has_infoframe);
> >>
> >> PIPE_CONF_CHECK_I(has_audio);
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> >> b/drivers/gpu/drm/i915/intel_drv.h
> >> index 3af20c1..50d680c 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -729,6 +729,12 @@ struct intel_crtc_state {
> >>
> >> /* bitmask of visible planes (enum plane_id) */
> >> u8 active_planes;
> >> +
> >> + /* HDMI scrambling status */
> >> + bool hdmi_scrambling;
> >> +
> >> + /* HDMI High TMDS char rate ratio */
> >> + bool hdmi_high_tmds_clock_ratio;
> >> };
> >>
> >> struct intel_crtc {
> >> @@ -1623,6 +1629,10 @@ 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);
> >> +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..4585aa8 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 && IS_GEMINILAKE(dev_priv)) {
> >> + 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,56 @@ static void intel_hdmi_destroy(struct drm_connector
> >> *connector)
> >> intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> >> }
> >>
> >> +/*
> >> + * This function handles scrambling on HDMI 2.0 capable sinks.
> >> + * If required clock rate is > 340 Mhz && scrambling is supported by sink
> >> + * it enables scrambling. This should be called before enabling the HDMI
> >> + * 2.0 port, as the sink can choose to disable the scrambling if it
> >> doesn't
> >> + * detect a scrambled clock within 100 ms.
> >> + */
> >> +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;
> >> +
> >> + if (!IS_GEMINILAKE(dev_priv))
> >> + return;
> > I'd still like to see this getting done uncoditionally. That way we
> > might get a bit of extra testing for the SCDC helper no matter what
> > platform the user happens to own.
> Sure, seems a good idea. I just added this hear to address the review
> comment from Ander.
> Also, I can move this IS_GLK condition to compute_config, and once we
> check this in compute_config
> we might never have check it again.
I thought you already had the check in there? Hmm. Looks that way to me.
> > I presume the spec says the sink will reset these to some default value
> > on HPD or something like that? But I'm not sure trusting them to get
> > this right is a good idea because reality often doesn't match the
> > specs. So the safer option IMO would be always tell the sink exactly
> > what we want.
> Yep, You are correct about the spec too, that it says if there is no
> scrambled clock post hpd_out for
> 100ms, the sink should disable scrambling. But connected modeset
> scenarios need our attention, where
> for example we are switching mode from, 4k@60 (scrambling reqd) ->
> 1080@60 (scrambling not reqd).
> For such cases too, we have to tell sink to disable scrambling.
> >
> >> +
> >> + DRM_DEBUG_KMS("Setting sink scrambling for enc:%s connector:%s\n",
> >> + intel_encoder->base.name, connector->name);
> >> +
> >> + if (config->hdmi_high_tmds_clock_ratio) {
> >> + /* Set TMDS bit clock ratio to 1/40 or 1/10 */
> >> + ret = drm_scdc_set_high_tmds_clock_ratio(adptr, enable);
> >> + if (!ret) {
> >> + DRM_ERROR("Set TMDS ratio failed\n");
> >> + return;
> >> + }
> >> + }
> >> +
> >> + if (config->hdmi_scrambling) {
> >> + /* Enable/disable sink scrambling */
> >> + ret = drm_scdc_set_scrambling(adptr, enable);
> >> + if (!ret) {
> >> + DRM_ERROR("Set sink scrambling failed\n");
> >> + return;
> >> + }
> >> + }
> >> +
> >> + DRM_DEBUG_KMS("sink scrambling handled\n");
> >> +}
> >> +
> >> static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> >> enum port port)
> >> {
> >> --
> >> 1.9.1
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
