Re: [Intel-gfx] [PATCH] /drm/i915/hdmi: SCDC Scrambling enable without CTS mode
On Tue, Oct 09, 2018 at 04:30:45PM -0700, Clint Taylor wrote:
>
>
> On 10/08/2018 03:33 AM, Ville Syrjälä wrote:
> > On Fri, Oct 05, 2018 at 03:18:44PM -0700, clinton.a.tay...@intel.com wrote:
> >> From: Clint Taylor
> >>
> >> Setting the SCDC scrambling CTS mode causes HDMI Link Layer protocol tests
> >> HF1-12 and HF1-13 to fail. Added "Source Shall" entries from SCDC
> >> section before enabling scrambling.
> >>
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107895
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107896
> >> Signed-off-by: Clint Taylor
> >> ---
> >> drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
> >> drivers/gpu/drm/i915/intel_hdmi.c | 8
> >> 2 files changed, 11 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> >> b/drivers/gpu/drm/i915/intel_ddi.c
> >> index 9e82281..a1b877f 100644
> >> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> @@ -1872,7 +1872,7 @@ void intel_ddi_enable_transcoder_func(const struct
> >> intel_crtc_state *crtc_state)
> >>temp |= TRANS_DDI_MODE_SELECT_DVI;
> >>
> >>if (crtc_state->hdmi_scrambling)
> >> - temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
> >> + temp |= TRANS_DDI_HDMI_SCRAMBLING;
> >>if (crtc_state->hdmi_high_tmds_clock_ratio)
> >>temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
> >>} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
> >> @@ -3394,8 +3394,8 @@ void intel_ddi_get_config(struct intel_encoder
> >> *encoder,
> >>if (intel_dig_port->infoframe_enabled(encoder, pipe_config))
> >>pipe_config->has_infoframe = true;
> >>
> >> - if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
> >> - TRANS_DDI_HDMI_SCRAMBLING_MASK)
> >> + if ((temp & TRANS_DDI_HDMI_SCRAMBLING) ==
> >> + TRANS_DDI_HDMI_SCRAMBLING)
> > It's a single bit so 'temp & TRANS_DDI_HDMI_SCRAMBLING' will do.
> I will optimize the statement.
> > The spec isn't particularly clear about the CTS enable bit, but judging
> > from the name I guess you should only enable it when doing compliance
> > testing.
> Section 6.1.2.4.1 of the HDMI 2.1 specification contains some
> information about the CTS testing. In normal video transmission there is
> one SSCP transmitted per field. Of course the HDMI 2.0 CTS doesn't
> mention a need for per line SSCP's that this bit enables.
> >>pipe_config->hdmi_scrambling = true;
> >>if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
> >>pipe_config->hdmi_high_tmds_clock_ratio = true;
> >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> >> b/drivers/gpu/drm/i915/intel_hdmi.c
> >> index 454f570..d181d67 100644
> >> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >> @@ -2148,6 +2148,14 @@ bool intel_hdmi_handle_sink_scrambling(struct
> >> intel_encoder *encoder,
> >> connector->base.id, connector->name,
> >> yesno(scrambling), high_tmds_clock_ratio ? 40 : 10);
> >>
> >> + /* SCDC source version 10.4.1.2 */
> >> + if (drm_scdc_writeb(adapter, SCDC_SOURCE_VERSION, 0x01) < 0)
> >> + DRM_DEBUG_KMS("Unable to set SCDC Source Version register\n");
> > These look unrelated to the scrambler fix, so should be a separate patch.
> Agreed. There are more fixes on the way to correct enable/disable
> scrambling and the SCDC registers.
> > I don't think the spec section numbers are particularly helpful without
> > some indication as to which specification they refer to.
> Will add specification version information.
> >> +
> >> + /* Clear SCDC CONFIG_0 10.4.1.6 - RR_Enable Polling Only */
> >> + if (drm_scdc_writeb(adapter, SCDC_CONFIG_0, 0x00) < 0)
> >> + DRM_DEBUG_KMS("Unable to set SCDC CONFIG_0 register\n");
> > The spec is unfortunately vague about this stuff. It sort of implies
> > that polling is optional, but then it says that if either source or sink
> > doesn't set the RR bit then polling must be used, which to me seems
> > like polling is in fact mandatory.
> I'm experimenting with an HDMI hotplug handler specifically for HDMI 2.0
> sinks. I would prefer not to wake up every 250ms, powering up the DDC
> lines, doing a single byte read, and sleeping again.
> >
> > The spec allows for a max polling interval of 250 ms. I don't particulary
> > cherish waking up every 250ms whenver a HDMI 2.0 sink is hooked up. I
> > guess maybe we could limit it to times when the link is actually active,
> > but it still feels very wasteful to poll for something that should
> > basically never happen.
> >
> > This is rather like the eDP dpcd polling when hpd isn't support.
> > Except IIRC the eDP polling is actually opitonal and we haven't
> > bothered to implement it. But I'm not even sure whether there are
> > any machines w/o eDP hpd
Re: [Intel-gfx] [PATCH] /drm/i915/hdmi: SCDC Scrambling enable without CTS mode
On 10/08/2018 03:33 AM, Ville Syrjälä wrote:
On Fri, Oct 05, 2018 at 03:18:44PM -0700, clinton.a.tay...@intel.com wrote:
From: Clint Taylor
Setting the SCDC scrambling CTS mode causes HDMI Link Layer protocol tests
HF1-12 and HF1-13 to fail. Added "Source Shall" entries from SCDC
section before enabling scrambling.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107895
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107896
Signed-off-by: Clint Taylor
---
drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
drivers/gpu/drm/i915/intel_hdmi.c | 8
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9e82281..a1b877f 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1872,7 +1872,7 @@ void intel_ddi_enable_transcoder_func(const struct
intel_crtc_state *crtc_state)
temp |= TRANS_DDI_MODE_SELECT_DVI;
if (crtc_state->hdmi_scrambling)
- temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
+ temp |= TRANS_DDI_HDMI_SCRAMBLING;
if (crtc_state->hdmi_high_tmds_clock_ratio)
temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
@@ -3394,8 +3394,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
if (intel_dig_port->infoframe_enabled(encoder, pipe_config))
pipe_config->has_infoframe = true;
- if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
- TRANS_DDI_HDMI_SCRAMBLING_MASK)
+ if ((temp & TRANS_DDI_HDMI_SCRAMBLING) ==
+ TRANS_DDI_HDMI_SCRAMBLING)
It's a single bit so 'temp & TRANS_DDI_HDMI_SCRAMBLING' will do.
I will optimize the statement.
The spec isn't particularly clear about the CTS enable bit, but judging
from the name I guess you should only enable it when doing compliance
testing.
Section 6.1.2.4.1 of the HDMI 2.1 specification contains some
information about the CTS testing. In normal video transmission there is
one SSCP transmitted per field. Of course the HDMI 2.0 CTS doesn't
mention a need for per line SSCP's that this bit enables.
pipe_config->hdmi_scrambling = true;
if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
pipe_config->hdmi_high_tmds_clock_ratio = true;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 454f570..d181d67 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2148,6 +2148,14 @@ bool intel_hdmi_handle_sink_scrambling(struct
intel_encoder *encoder,
connector->base.id, connector->name,
yesno(scrambling), high_tmds_clock_ratio ? 40 : 10);
+ /* SCDC source version 10.4.1.2 */
+ if (drm_scdc_writeb(adapter, SCDC_SOURCE_VERSION, 0x01) < 0)
+ DRM_DEBUG_KMS("Unable to set SCDC Source Version register\n");
These look unrelated to the scrambler fix, so should be a separate patch.
Agreed. There are more fixes on the way to correct enable/disable
scrambling and the SCDC registers.
I don't think the spec section numbers are particularly helpful without
some indication as to which specification they refer to.
Will add specification version information.
+
+ /* Clear SCDC CONFIG_0 10.4.1.6 - RR_Enable Polling Only */
+ if (drm_scdc_writeb(adapter, SCDC_CONFIG_0, 0x00) < 0)
+ DRM_DEBUG_KMS("Unable to set SCDC CONFIG_0 register\n");
The spec is unfortunately vague about this stuff. It sort of implies
that polling is optional, but then it says that if either source or sink
doesn't set the RR bit then polling must be used, which to me seems
like polling is in fact mandatory.
I'm experimenting with an HDMI hotplug handler specifically for HDMI 2.0
sinks. I would prefer not to wake up every 250ms, powering up the DDC
lines, doing a single byte read, and sleeping again.
The spec allows for a max polling interval of 250 ms. I don't particulary
cherish waking up every 250ms whenver a HDMI 2.0 sink is hooked up. I
guess maybe we could limit it to times when the link is actually active,
but it still feels very wasteful to poll for something that should
basically never happen.
This is rather like the eDP dpcd polling when hpd isn't support.
Except IIRC the eDP polling is actually opitonal and we haven't
bothered to implement it. But I'm not even sure whether there are
any machines w/o eDP hpd hooked up.
Anyway, back to the patch itself. It seems to me that we should
probably be configuring this stuff during detect rather than
during crtc enable.
The SCDC scramble_enable bit must be enabled within 100ms of sending
scrambled data. Maybe compute_config would work.
-Clint
+
/* Set TMDS bit clock ratio to 1/40 or 1/10, and enable/disable
Re: [Intel-gfx] [PATCH] /drm/i915/hdmi: SCDC Scrambling enable without CTS mode
On Fri, Oct 05, 2018 at 03:18:44PM -0700, clinton.a.tay...@intel.com wrote:
> From: Clint Taylor
>
> Setting the SCDC scrambling CTS mode causes HDMI Link Layer protocol tests
> HF1-12 and HF1-13 to fail. Added "Source Shall" entries from SCDC
> section before enabling scrambling.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107895
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107896
> Signed-off-by: Clint Taylor
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
> drivers/gpu/drm/i915/intel_hdmi.c | 8
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 9e82281..a1b877f 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1872,7 +1872,7 @@ void intel_ddi_enable_transcoder_func(const struct
> intel_crtc_state *crtc_state)
> temp |= TRANS_DDI_MODE_SELECT_DVI;
>
> if (crtc_state->hdmi_scrambling)
> - temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
> + temp |= TRANS_DDI_HDMI_SCRAMBLING;
> if (crtc_state->hdmi_high_tmds_clock_ratio)
> temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
> } else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
> @@ -3394,8 +3394,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> if (intel_dig_port->infoframe_enabled(encoder, pipe_config))
> pipe_config->has_infoframe = true;
>
> - if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
> - TRANS_DDI_HDMI_SCRAMBLING_MASK)
> + if ((temp & TRANS_DDI_HDMI_SCRAMBLING) ==
> + TRANS_DDI_HDMI_SCRAMBLING)
It's a single bit so 'temp & TRANS_DDI_HDMI_SCRAMBLING' will do.
The spec isn't particularly clear about the CTS enable bit, but judging
from the name I guess you should only enable it when doing compliance
testing.
> pipe_config->hdmi_scrambling = true;
> if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
> pipe_config->hdmi_high_tmds_clock_ratio = true;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 454f570..d181d67 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2148,6 +2148,14 @@ bool intel_hdmi_handle_sink_scrambling(struct
> intel_encoder *encoder,
> connector->base.id, connector->name,
> yesno(scrambling), high_tmds_clock_ratio ? 40 : 10);
>
> + /* SCDC source version 10.4.1.2 */
> + if (drm_scdc_writeb(adapter, SCDC_SOURCE_VERSION, 0x01) < 0)
> + DRM_DEBUG_KMS("Unable to set SCDC Source Version register\n");
These look unrelated to the scrambler fix, so should be a separate patch.
I don't think the spec section numbers are particularly helpful without
some indication as to which specification they refer to.
> +
> + /* Clear SCDC CONFIG_0 10.4.1.6 - RR_Enable Polling Only */
> + if (drm_scdc_writeb(adapter, SCDC_CONFIG_0, 0x00) < 0)
> + DRM_DEBUG_KMS("Unable to set SCDC CONFIG_0 register\n");
The spec is unfortunately vague about this stuff. It sort of implies
that polling is optional, but then it says that if either source or sink
doesn't set the RR bit then polling must be used, which to me seems
like polling is in fact mandatory.
The spec allows for a max polling interval of 250 ms. I don't particulary
cherish waking up every 250ms whenver a HDMI 2.0 sink is hooked up. I
guess maybe we could limit it to times when the link is actually active,
but it still feels very wasteful to poll for something that should
basically never happen.
This is rather like the eDP dpcd polling when hpd isn't support.
Except IIRC the eDP polling is actually opitonal and we haven't
bothered to implement it. But I'm not even sure whether there are
any machines w/o eDP hpd hooked up.
Anyway, back to the patch itself. It seems to me that we should
probably be configuring this stuff during detect rather than
during crtc enable.
> +
> /* Set TMDS bit clock ratio to 1/40 or 1/10, and enable/disable
> scrambling */
> return drm_scdc_set_high_tmds_clock_ratio(adapter,
> high_tmds_clock_ratio) &&
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] /drm/i915/hdmi: SCDC Scrambling enable without CTS mode
From: Clint Taylor
Setting the SCDC scrambling CTS mode causes HDMI Link Layer protocol tests
HF1-12 and HF1-13 to fail. Added "Source Shall" entries from SCDC
section before enabling scrambling.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107895
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107896
Signed-off-by: Clint Taylor
---
drivers/gpu/drm/i915/intel_ddi.c | 6 +++---
drivers/gpu/drm/i915/intel_hdmi.c | 8
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9e82281..a1b877f 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1872,7 +1872,7 @@ void intel_ddi_enable_transcoder_func(const struct
intel_crtc_state *crtc_state)
temp |= TRANS_DDI_MODE_SELECT_DVI;
if (crtc_state->hdmi_scrambling)
- temp |= TRANS_DDI_HDMI_SCRAMBLING_MASK;
+ temp |= TRANS_DDI_HDMI_SCRAMBLING;
if (crtc_state->hdmi_high_tmds_clock_ratio)
temp |= TRANS_DDI_HIGH_TMDS_CHAR_RATE;
} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
@@ -3394,8 +3394,8 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
if (intel_dig_port->infoframe_enabled(encoder, pipe_config))
pipe_config->has_infoframe = true;
- if ((temp & TRANS_DDI_HDMI_SCRAMBLING_MASK) ==
- TRANS_DDI_HDMI_SCRAMBLING_MASK)
+ if ((temp & TRANS_DDI_HDMI_SCRAMBLING) ==
+ TRANS_DDI_HDMI_SCRAMBLING)
pipe_config->hdmi_scrambling = true;
if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
pipe_config->hdmi_high_tmds_clock_ratio = true;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
b/drivers/gpu/drm/i915/intel_hdmi.c
index 454f570..d181d67 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2148,6 +2148,14 @@ bool intel_hdmi_handle_sink_scrambling(struct
intel_encoder *encoder,
connector->base.id, connector->name,
yesno(scrambling), high_tmds_clock_ratio ? 40 : 10);
+ /* SCDC source version 10.4.1.2 */
+ if (drm_scdc_writeb(adapter, SCDC_SOURCE_VERSION, 0x01) < 0)
+ DRM_DEBUG_KMS("Unable to set SCDC Source Version register\n");
+
+ /* Clear SCDC CONFIG_0 10.4.1.6 - RR_Enable Polling Only */
+ if (drm_scdc_writeb(adapter, SCDC_CONFIG_0, 0x00) < 0)
+ DRM_DEBUG_KMS("Unable to set SCDC CONFIG_0 register\n");
+
/* Set TMDS bit clock ratio to 1/40 or 1/10, and enable/disable
scrambling */
return drm_scdc_set_high_tmds_clock_ratio(adapter,
high_tmds_clock_ratio) &&
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
