[Intel-gfx] [PATCH v8 5/6] drm/i915: enable scrambling
Sharma, Shashank
shashank.sharma at intel.com
Wed Mar 8 17:26:45 UTC 2017
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.sharma at 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 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 at 60 (scrambling reqd) ->
1080 at 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
More information about the Intel-gfx
mailing list