On Thu, Feb 02, 2017 at 04:15:21PM +0530, Sharma, Shashank wrote:
> 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 ?
intel_enable_ddi() is where we presumbly enable the port,
so maybe there.
>
> - 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
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
