diff mbox

[01/15] drm/i915: fixup 12bpc hdmi dotclock handling

Message ID 1366363487-15926-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter April 19, 2013, 9:24 a.m. UTC
We need to multiply the hdmi port dotclock by 1.5x since it's not
really a dotclock, but the 10/8 encoding bitclock divided by 10.

Also add correct limit checks for the dotclock and reject modes which
don't fit. HDMI 1.4 would allow more, but our hw doesn't support that
unfortunately :(

Somehow I suspect 12bpc hdmi output never really worked - we really
need an i-g-t testcase to check all the different pixel modes and
outputs.

v2: Fixup the adjusted port clock handling - we need to make sure that
the fdi link code still gets the real pixelclock.

v3: g4x/vlv don't support 12bpc hdmi output so drop the bogus comment.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Ville Syrjälä April 23, 2013, 3:02 p.m. UTC | #1
On Fri, Apr 19, 2013 at 11:24:33AM +0200, Daniel Vetter wrote:
> We need to multiply the hdmi port dotclock by 1.5x since it's not
> really a dotclock, but the 10/8 encoding bitclock divided by 10.
> 
> Also add correct limit checks for the dotclock and reject modes which
> don't fit. HDMI 1.4 would allow more, but our hw doesn't support that
> unfortunately :(
> 
> Somehow I suspect 12bpc hdmi output never really worked - we really
> need an i-g-t testcase to check all the different pixel modes and
> outputs.
> 
> v2: Fixup the adjusted port clock handling - we need to make sure that
> the fdi link code still gets the real pixelclock.
> 
> v3: g4x/vlv don't support 12bpc hdmi output so drop the bogus comment.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Seems to match what the spec says. Although I'm getting confused about
all the clocks we have. Somehow I'd prefer to store the pixel clock in
the mode, and move the link clock out, but perhaps that makes things
more messy elsewhere...

Also I'm confused about the 225 MHz limit. That seems to be specific to
PCH platforms. My g4x docs say that HDMI limit is 165 MHz. Also
intel_limits_g4x_hdmi() has dot.max of 400 MHz which seems to contradict
the docs (400 MHz is correct for the DAC apparently). But then again the
docs also say that the p1_slow/p2_fast switchover point is 165 MHz, so
if the max is 165 MHz too, there would never be any need to use p2_fast
with HDMI. The more I read the docs, the more confused I get.

> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 53ce8a5..a8273c7 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -783,6 +783,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>  	struct drm_device *dev = encoder->base.dev;
>  	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> +	int clock_12bpc = pipe_config->requested_mode.clock * 3 / 2;
>  
>  	if (intel_hdmi->color_range_auto) {
>  		/* See CEA-861-E - 5.1 Default Encoding Parameters */
> @@ -802,16 +803,28 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  	/*
>  	 * HDMI is either 12 or 8, so if the display lets 10bpc sneak
>  	 * through, clamp it down. Note that g4x/vlv don't support 12bpc hdmi
> -	 * outputs.
> +	 * outputs. We also need to check that the higher clock still fits
> +	 * within limits.
>  	 */
> -	if (pipe_config->pipe_bpp > 8*3 && HAS_PCH_SPLIT(dev)) {
> +	if (pipe_config->pipe_bpp > 8*3 && clock_12bpc < 225000
> +	    && HAS_PCH_SPLIT(dev)) {
>  		DRM_DEBUG_KMS("forcing bpc to 12 for HDMI\n");
>  		pipe_config->pipe_bpp = 12*3;
> +
> +		/* Need to adjust the port link by 1.5x for 12bpc. */
> +		adjusted_mode->clock = clock_12bpc;
> +		pipe_config->pixel_target_clock =
> +			pipe_config->requested_mode.clock;
>  	} else {
>  		DRM_DEBUG_KMS("forcing bpc to 8 for HDMI\n");
>  		pipe_config->pipe_bpp = 8*3;
>  	}
>  
> +	if (adjusted_mode->clock > 225000) {
> +		DRM_DEBUG_KMS("too high HDMI clock, rejecting mode\n");
> +		return false;
> +	}
> +
>  	return true;
>  }
>  
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter April 23, 2013, 3:37 p.m. UTC | #2
On Tue, Apr 23, 2013 at 5:02 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Fri, Apr 19, 2013 at 11:24:33AM +0200, Daniel Vetter wrote:
>> We need to multiply the hdmi port dotclock by 1.5x since it's not
>> really a dotclock, but the 10/8 encoding bitclock divided by 10.
>>
>> Also add correct limit checks for the dotclock and reject modes which
>> don't fit. HDMI 1.4 would allow more, but our hw doesn't support that
>> unfortunately :(
>>
>> Somehow I suspect 12bpc hdmi output never really worked - we really
>> need an i-g-t testcase to check all the different pixel modes and
>> outputs.
>>
>> v2: Fixup the adjusted port clock handling - we need to make sure that
>> the fdi link code still gets the real pixelclock.
>>
>> v3: g4x/vlv don't support 12bpc hdmi output so drop the bogus comment.
>>
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Seems to match what the spec says. Although I'm getting confused about
> all the clocks we have. Somehow I'd prefer to store the pixel clock in
> the mode, and move the link clock out, but perhaps that makes things
> more messy elsewhere...

Yeah, I think we need to switch over to storing the adjusted pixel
clok in adjuste_mode->clock and keep the link clock in a separate
field in the pipe config. The current code is just a mess.

I haven't done this yet since I need some more clarity first, so that
we don't horribly break eDP ...

> Also I'm confused about the 225 MHz limit. That seems to be specific to
> PCH platforms. My g4x docs say that HDMI limit is 165 MHz. Also
> intel_limits_g4x_hdmi() has dot.max of 400 MHz which seems to contradict
> the docs (400 MHz is correct for the DAC apparently). But then again the
> docs also say that the p1_slow/p2_fast switchover point is 165 MHz, so
> if the max is 165 MHz too, there would never be any need to use p2_fast
> with HDMI. The more I read the docs, the more confused I get.

Yeah, it's a complete disaster. My thinking with the 225 MHz clock is that:
- 12 bpc is a hdmi-only feature, so we can forget about any fancy DVI cases
- hdmi spec up to 1.2 says 225MHz is the upper limit of the 8/10 symbol clock.

So I've figured by limiting things to the 1.2 hdmi spec I could avoid
trying to untangle this mess and trying to reconcile our code with
Bspec. I'm cheap, I know ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 53ce8a5..a8273c7 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -783,6 +783,7 @@  bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
+	int clock_12bpc = pipe_config->requested_mode.clock * 3 / 2;
 
 	if (intel_hdmi->color_range_auto) {
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
@@ -802,16 +803,28 @@  bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 	/*
 	 * HDMI is either 12 or 8, so if the display lets 10bpc sneak
 	 * through, clamp it down. Note that g4x/vlv don't support 12bpc hdmi
-	 * outputs.
+	 * outputs. We also need to check that the higher clock still fits
+	 * within limits.
 	 */
-	if (pipe_config->pipe_bpp > 8*3 && HAS_PCH_SPLIT(dev)) {
+	if (pipe_config->pipe_bpp > 8*3 && clock_12bpc < 225000
+	    && HAS_PCH_SPLIT(dev)) {
 		DRM_DEBUG_KMS("forcing bpc to 12 for HDMI\n");
 		pipe_config->pipe_bpp = 12*3;
+
+		/* Need to adjust the port link by 1.5x for 12bpc. */
+		adjusted_mode->clock = clock_12bpc;
+		pipe_config->pixel_target_clock =
+			pipe_config->requested_mode.clock;
 	} else {
 		DRM_DEBUG_KMS("forcing bpc to 8 for HDMI\n");
 		pipe_config->pipe_bpp = 8*3;
 	}
 
+	if (adjusted_mode->clock > 225000) {
+		DRM_DEBUG_KMS("too high HDMI clock, rejecting mode\n");
+		return false;
+	}
+
 	return true;
 }