On Thu, 2017-02-23 at 10:01 +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 2/22/2017 10:59 PM, Ville Syrjälä wrote:
> > On Wed, Feb 22, 2017 at 06:48:30PM +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
> > >
> > > Signed-off-by: Shashank Sharma <shashank.sha...@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_reg.h | 4 ++
> > > drivers/gpu/drm/i915/intel_ddi.c | 28 ++++++++++
> > > drivers/gpu/drm/i915/intel_drv.h | 14 +++++
> > > drivers/gpu/drm/i915/intel_hdmi.c | 108
> > > ++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 154 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 141a5c1..81cf10b 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7819,7 +7819,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 cd6fedd..bd8293d 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_handle_source_scrambling(
> > > + intel_encoder,
> > > + &intel_crtc->config->base.adjusted_mode, temp);
> > > } else if (type == INTEL_OUTPUT_ANALOG) {
> > > temp |= TRANS_DDI_MODE_SELECT_FDI;
> > > temp |= (intel_crtc->config->fdi_lanes - 1) << 1;
> > > @@ -1843,6 +1848,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.
> > > @@ -1875,6 +1895,14 @@ 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) {
> > > + 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 821c57c..c7262d7 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -691,6 +691,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 {
> > > @@ -1609,6 +1615,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 drm_display_mode *mode,
> > > + uint32_t 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 a580de8..c44beee 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>
> > > @@ -1795,6 +1796,113 @@ 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_scdc *scdc = &connector->display_info.hdmi.scdc;
> > > + struct drm_scrambling *scrambling = &scdc->scrambling;
> > > + struct drm_display_mode *mode = &config->base.adjusted_mode;
> > > + struct drm_i915_private *dev_priv = connector->dev->dev_private;
> > > + struct i2c_adapter *adptr = intel_gmbus_get_adapter(dev_priv,
There's only two letters missing. Maybe just spell adapter.
> > > + intel_hdmi->ddc_bus);
> > > +
> > > + 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 (mode->clock > 340000) {
> > > + /* Set TMDS bit clock ratio to 1/40 */
> > > + config->hdmi_high_tmds_clock_ratio =
> > > + drm_scdc_set_high_tmds_clock_ratio(adptr);
> >
> > You're not allowed to muck with the state anymore. All state computation
> > should happen in the .compute_config hook.
>
> ok, let me re-arrange this function in such a way that I do the config
> update part in compute_config
> > > + if (!config->hdmi_high_tmds_clock_ratio) {
> > > + DRM_ERROR("Set high TMDS ratio failed\n");
> > > + return;
> > > + }
> > > +
> > > + /* Enable sink scrambling */
> > > + config->hdmi_scrambling =
> > > + drm_scdc_enable_scrambling(adptr);
> > > + if (!config->hdmi_scrambling) {
> > > + DRM_ERROR("Can't enable sink scrambling\n");
> > > + return;
> > > + }
> > > + }
> > > +
> > > + /* Few sinks support scrambling at clocks <=340 MHz too */
> > > + if (!config->hdmi_scrambling && scrambling->low_rates) {
> > > + config->hdmi_scrambling =
> > > + drm_scdc_enable_scrambling(adptr);
> > > + if (!config->hdmi_scrambling)
> > > + DRM_ERROR("Can't enable sink scrambling\n");
> > > + }
> > > +
> > > + return;
> > > + }
> > > +
> > > + if (config->hdmi_high_tmds_clock_ratio) {
> > > + /* Set TMDS bit clock ratio back to 1/10 */
> > > + config->hdmi_high_tmds_clock_ratio =
> > > + !(drm_scdc_clear_high_tmds_clock_ratio(adptr));
> > > + if (config->hdmi_high_tmds_clock_ratio)
> > > + DRM_ERROR("Reset high TMDS ratio failed\n");
> > > + }
> > > +
> > > + if (config->hdmi_scrambling) {
> > > + /* Disable sink scrambling */
> > > + config->hdmi_scrambling = !(drm_scdc_disable_scrambling(adptr));
> > > + if (config->hdmi_scrambling)
> > > + DRM_ERROR("Disable sink scrambling failed\n");
> > > + }
> > > +}
> > > +
> > > +static inline uint32_t _intel_hdmi_set_source_scrambling(uint32_t
> > > hdmi_config)
> > > +{
> > > + return hdmi_config |= (TRANS_DDI_HDMI_SCRAMBLING |
> > > + TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ |
> > > + TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE);
> > > +}
> > > +
> > > +uint32_t
> > > +intel_hdmi_handle_source_scrambling(struct intel_encoder *intel_encoder,
> > > + struct drm_display_mode *mode, 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;
> > > + struct drm_hdmi_info *hdmi_info = &connector->display_info.hdmi;
> > > + struct drm_scrambling *scrambling = &hdmi_info->scdc.scrambling;
> > > +
> > > + 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 (mode->clock <= 340000) {
> > > + /* Few sinks support scrambling at rate < 340 MHz too */
> > > + if (scrambling->low_rates)
> > > + hdmi_config =
> > > + _intel_hdmi_set_source_scrambling(hdmi_config);
> > > + return hdmi_config;
> > > + }
> > > +
> > > + /* Scrambling or not, if clock > 340 MHz, set high char rate */
> > > + hdmi_config |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
> > > +
> > > + if (scrambling->supported)
> > > + hdmi_config = _intel_hdmi_set_source_scrambling(hdmi_config);
> > > +
> > > + return hdmi_config;
> > > +}
> >
> > Seems overly complicated to me. It could just be something simple like:
> >
> > void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc)
> > {
> > ...
> > if (config->hdmi_scrambling)
> > temp |= TRANS_DDI_HDMI_SCRAMBLING;
> >
> > if (config->whatever)
> > temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
> > ...
> > }
>
> I know it appears like complicated, but actually the requirement is
> complicated:
> - if clock is > 340 Mhz, set scrambling && set_high_tmds_clock_ratio
> - if clock is < 340 Mhz, but if monitor supports scrambling at lower
> clocks, set scrambling only (not tmds_clock_ratio)
> - to set scrambling, we have to first check if scrambling is supported
> in monitor
> So to meet all these condition, you will end up with something like
> what's implemented above.
> Do you see a better way ?
Put that intel_hdmi_compute_config() so you check it only once, i.e.:
if (clock > 340 || scrambling->low_rates)
config->hdmi_scrambling = true;
if (clock > 340)
config->hdmi_high_tmds = true;
Then you can do just as Ville said above in intel_ddi_enable_transcoder_func().
And also for the sink part, no need to check the input values again:
if (config->hdmi_high_tmds)
if (!drm_scdc_set_high_tmds_clock_ratio(adapter, enable))
DRM_ERROR();
if (config->hdmi_scrambing)
if (!drm_scdc_set_scrambling(adapter, enable))
DRM_ERROR();
At least that's how I'd interpret what Ville wrote.
Ander
>
> - Shashank
> >
> > > +
> > > static u8 intel_hdmi_ddc_pin(struct drm_i915_private *dev_priv,
> > > enum port port)
> > > {
> > > --
> > > 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
