Re: [PATCH 5/6] drm/i915: enable scrambling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Regards

Shashank


On 2/2/2017 3:32 PM, Ville Syrjälä wrote:
On Thu, Feb 02, 2017 at 11:23:19AM +0530, Sharma, Shashank wrote:
Regards

Shashank


On 2/1/2017 10:06 PM, Ville Syrjälä wrote:
On Wed, Feb 01, 2017 at 06:14:40PM +0530, Shashank Sharma wrote:
Geminilake platform has 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.

Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
---
   drivers/gpu/drm/i915/i915_reg.h   |  2 ++
   drivers/gpu/drm/i915/intel_ddi.c  |  5 +++++
   drivers/gpu/drm/i915/intel_drv.h  |  2 ++
   drivers/gpu/drm/i915/intel_hdmi.c | 42 +++++++++++++++++++++++++++++++++++++++
   4 files changed, 51 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 495b789..cc85892 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7807,6 +7807,8 @@ enum {
   #define  TRANS_DDI_EDP_INPUT_C_ONOFF	(6<<12)
   #define  TRANS_DDI_DP_VC_PAYLOAD_ALLOC	(1<<8)
   #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 9a9a670..aea81ce 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1278,6 +1278,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_check_scrambling(intel_encoder,
+				&intel_crtc->config->base.adjusted_mode);
+		}
   	} else if (type == INTEL_OUTPUT_ANALOG) {
   		temp |= TRANS_DDI_MODE_SELECT_FDI;
   		temp |= (intel_crtc->config->fdi_lanes - 1) << 1;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 393f243..aafce7f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1588,6 +1588,8 @@ 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_check_scrambling(struct intel_encoder *intel_encoder,
+				struct drm_display_mode *mode);
   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 ebae2bd..92dd9bc 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1795,6 +1795,48 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
   	intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
   }
+static void
+intel_hdmi_enable_scrambling(struct drm_connector *connector)
+{
+	struct drm_i915_private *dev_priv = connector->dev->dev_private;
+	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+	struct i2c_adapter *adapter =
+			intel_gmbus_get_adapter(dev_priv, intel_hdmi->ddc_bus);
+
+	if (!drm_enable_scrambling(connector, adapter, true))
+		DRM_ERROR("Request to enable scrambling failed\n");
+}
I don't like hiding this somewhere deep like this. It should be
somewhere much higher up.
Why ? All we need to do here is enable two bits in transcoder control
register, which is already being
programmed in a calling function, so I dont see the use case, but I
might be missing some bigger picture.
Can you please elaborate on this ?
We're talking to the display here, which is rather surprising to happen
from a function thing that on the first glance doesn't even seem to
touch the hardware.
I kind of agree with this, and I am also tempted now to shift this call upwards, but I want to
keep this close to port enabling as well, as the HDMI 2.0 spec says:
"Max time period between a source enables scrambling on monitor, and starts sending scrambled
  video should be 100ms" else the sink might choose to disable scrambling.

Considering this, where do you think would be right place to enabled scrambling on monitor ?

- Shashank
And I'm thinkign we might want to track the scrambler state
in the crtc state.
Yes, that's a pretty good way to track dynamic status of scrambler, do
you think we should add this in
drm_crtc_state itself ?
I'd just start with intel specific state and later we can see what
everyone else wants to do and potentially unify a bit.

- Shashank
+
+uint32_t intel_hdmi_check_scrambling(struct intel_encoder *intel_encoder,
+				struct drm_display_mode *mode)
+{
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
+	struct drm_connector *connector = &intel_hdmi->attached_connector->base;
+	struct drm_hdmi_info *hdmi_info = &connector->display_info.hdmi_info;
+	uint32_t hdmi_config = 0;
+
+	DRM_DEBUG_KMS("Checking scrambling for enc:%s connector:%s\n",
+			intel_encoder->base.name, connector->name);
+
+	if (mode->clock < 340000) {
+		if (hdmi_info->scr_info.low_clocks) {
+			intel_hdmi_enable_scrambling(connector);
+			hdmi_config |= TRANS_DDI_HDMI_SCRAMBLING;
+		}
+		return hdmi_config;
+	}
+
+	/* Enable scrambling for clocks > 340M */
+	if (hdmi_info->scr_info.supported) {
+		intel_hdmi_enable_scrambling(connector);
+		hdmi_config |= TRANS_DDI_HDMI_SCRAMBLING;
+	}
+
+	/* Scrambling or not, if clock > 340M, set high char rate */
+	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)
   {
--
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux