Hi,
this is a patch series to fix the breakages of eDP output on HP Z1
desktop machine with IvyBridge since 3.6 kernel.
The first patch is identical as what I already sent yesterday, reverting
a bogus check in the dp train loop. The second one is the patch I cooked
today, for using the correct pixel clock for the fixed eDP panel.
I guess the second patch won't be applied cleanly to the old kernels,
so it'd need to be sent individually to stable kernel if needed.
thanks,
Takashi
The eDP output on HP Z1 is still broken when X is started even after
fixing the infinite link-train loop. The regression was introduced in
3.6 kernel for cleaning up the mode clock handling code in intel_dp.c.
In the past, the clock of the reference mode was modified in
intel_dp_mode_fixup() in the case of eDP fixed clock, and this clock was
used for calculating in intel_dp_set_m_n(). This override was removed,
thus the wrong mode clock is used for the calculation, resulting in a
psychedelic smoking output in the end.
This patch corrects the clock to be used in the place.
Cc: <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
---
drivers/gpu/drm/i915/intel_dp.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7d250aa..ddbf50f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -819,6 +819,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
struct intel_link_m_n m_n;
int pipe = intel_crtc->pipe;
enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
+ int pixel_clock = mode->clock;
/*
* Find the lane count in the intel_encoder private
@@ -826,6 +827,13 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
intel_dp = enc_to_intel_dp(&intel_encoder->base);
+ if (intel_encoder->type == INTEL_OUTPUT_EDP) {
+ struct drm_display_mode *fixed_mode =
+ intel_dp->attached_connector->panel.fixed_mode;
+ if (fixed_mode)
+ pixel_clock = fixed_mode->clock;
+ }
+
if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
intel_encoder->type == INTEL_OUTPUT_EDP)
{
@@ -840,7 +848,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
* set up for 8-bits of R/G/B, or 3 bytes total.
*/
intel_link_compute_m_n(intel_crtc->bpp, lane_count,
- mode->clock, adjusted_mode->clock, &m_n);
+ pixel_clock, adjusted_mode->clock, &m_n);
if (IS_HASWELL(dev)) {
I915_WRITE(PIPE_DATA_M1(cpu_transcoder),
--
1.8.1.4
This reverts commit 0d71068835e2610576d369d6d4cbf90e0f802a71.
Not only that the commit introduces a bogus check (voltage_tries == 5
will never meet at the inserted code path), it may bring the driver into
an endless DP-link-train loop (and actually it does on HP Z1 with eDP).
We can add a better check for the intended purpose of that commit, but
let's revert the wrong "fix" at first.
Cc: <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
---
drivers/gpu/drm/i915/intel_dp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f61cb79..7d250aa 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1929,7 +1929,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
for (i = 0; i < intel_dp->lane_count; i++)
if ((intel_dp->train_set[i] & DP_TRAIN_MAX_SWING_REACHED) == 0)
break;
- if (i == intel_dp->lane_count && voltage_tries == 5) {
+ if (i == intel_dp->lane_count) {
++loop_tries;
if (loop_tries == 5) {
DRM_DEBUG_KMS("too many full retries, give up\n");
--
1.8.1.4
On Tue, Mar 12, 2013 at 04:32:28PM +0100, Takashi Iwai wrote:
> The eDP output on HP Z1 is still broken when X is started even after
> fixing the infinite link-train loop. The regression was introduced in
> 3.6 kernel for cleaning up the mode clock handling code in intel_dp.c.
>
> In the past, the clock of the reference mode was modified in
> intel_dp_mode_fixup() in the case of eDP fixed clock, and this clock was
> used for calculating in intel_dp_set_m_n(). This override was removed,
> thus the wrong mode clock is used for the calculation, resulting in a
> psychedelic smoking output in the end.
>
> This patch corrects the clock to be used in the place.
>
> Cc: <[email protected]>
> Signed-off-by: Takashi Iwai <[email protected]>
I truly hate this mess of dotclock vs portclock vs. whatever. Can you pls
apply a little bikeshed and use the existing intel_edp_target_clock like
in ironlake_set_m_n? And if you have the regressing commit a little
citation to assign the blame (it's probably me) would be good.
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
At Sun, 17 Mar 2013 23:12:03 +0100,
Daniel Vetter wrote:
>
> On Tue, Mar 12, 2013 at 04:32:28PM +0100, Takashi Iwai wrote:
> > The eDP output on HP Z1 is still broken when X is started even after
> > fixing the infinite link-train loop. The regression was introduced in
> > 3.6 kernel for cleaning up the mode clock handling code in intel_dp.c.
> >
> > In the past, the clock of the reference mode was modified in
> > intel_dp_mode_fixup() in the case of eDP fixed clock, and this clock was
> > used for calculating in intel_dp_set_m_n(). This override was removed,
> > thus the wrong mode clock is used for the calculation, resulting in a
> > psychedelic smoking output in the end.
> >
> > This patch corrects the clock to be used in the place.
> >
> > Cc: <[email protected]>
> > Signed-off-by: Takashi Iwai <[email protected]>
>
> I truly hate this mess of dotclock vs portclock vs. whatever. Can you pls
> apply a little bikeshed and use the existing intel_edp_target_clock like
> in ironlake_set_m_n? And if you have the regressing commit a little
> citation to assign the blame (it's probably me) would be good.
OK, the revised patch is below.
thanks,
Takashi
---
From: Takashi Iwai <[email protected]>
Subject: [PATCH v2] drm/i915: Use the fixed pixel clock for eDP in intel_dp_set_m_n()
The eDP output on HP Z1 is still broken when X is started even after
fixing the infinite link-train loop. The regression was introduced in
3.6 kernel for cleaning up the mode clock handling code in intel_dp.c
by the commit [71244653: drm/i915: adjusted_mode->clock in the dp
mode_fix].
In the past, the clock of the reference mode was modified in
intel_dp_mode_fixup() in the case of eDP fixed clock, and this clock was
used for calculating in intel_dp_set_m_n(). This override was removed,
thus the wrong mode clock is used for the calculation, resulting in a
psychedelic smoking output in the end.
This patch corrects the clock to be used in the place.
v1->v2: Use intel_edp_target_clock() for checking eDP fixed clock
instead of open code as in ironlake_set_m_n().
Cc: <[email protected]>
Signed-off-by: Takashi Iwai <[email protected]>
---
drivers/gpu/drm/i915/intel_dp.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 6f728e5..2606811 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -820,6 +820,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
struct intel_link_m_n m_n;
int pipe = intel_crtc->pipe;
enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
+ int target_clock;
/*
* Find the lane count in the intel_encoder private
@@ -835,13 +836,22 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
}
}
+ target_clock = mode->clock;
+ for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
+ if (intel_encoder->type == INTEL_OUTPUT_EDP) {
+ target_clock = intel_edp_target_clock(intel_encoder,
+ mode);
+ break;
+ }
+ }
+
/*
* Compute the GMCH and Link ratios. The '3' here is
* the number of bytes_per_pixel post-LUT, which we always
* set up for 8-bits of R/G/B, or 3 bytes total.
*/
intel_link_compute_m_n(intel_crtc->bpp, lane_count,
- mode->clock, adjusted_mode->clock, &m_n);
+ target_clock, adjusted_mode->clock, &m_n);
if (IS_HASWELL(dev)) {
I915_WRITE(PIPE_DATA_M1(cpu_transcoder),
--
1.8.2
On Mon, Mar 18, 2013 at 10:29:16AM +0100, Takashi Iwai wrote:
> At Sun, 17 Mar 2013 23:12:03 +0100,
> Daniel Vetter wrote:
> >
> > On Tue, Mar 12, 2013 at 04:32:28PM +0100, Takashi Iwai wrote:
> > > The eDP output on HP Z1 is still broken when X is started even after
> > > fixing the infinite link-train loop. The regression was introduced in
> > > 3.6 kernel for cleaning up the mode clock handling code in intel_dp.c.
> > >
> > > In the past, the clock of the reference mode was modified in
> > > intel_dp_mode_fixup() in the case of eDP fixed clock, and this clock was
> > > used for calculating in intel_dp_set_m_n(). This override was removed,
> > > thus the wrong mode clock is used for the calculation, resulting in a
> > > psychedelic smoking output in the end.
> > >
> > > This patch corrects the clock to be used in the place.
> > >
> > > Cc: <[email protected]>
> > > Signed-off-by: Takashi Iwai <[email protected]>
> >
> > I truly hate this mess of dotclock vs portclock vs. whatever. Can you pls
> > apply a little bikeshed and use the existing intel_edp_target_clock like
> > in ironlake_set_m_n? And if you have the regressing commit a little
> > citation to assign the blame (it's probably me) would be good.
>
> OK, the revised patch is below.
Picked up for -fixes, thanks for the patch.
-Daniel
>
>
> thanks,
>
> Takashi
>
> ---
> From: Takashi Iwai <[email protected]>
> Subject: [PATCH v2] drm/i915: Use the fixed pixel clock for eDP in intel_dp_set_m_n()
>
> The eDP output on HP Z1 is still broken when X is started even after
> fixing the infinite link-train loop. The regression was introduced in
> 3.6 kernel for cleaning up the mode clock handling code in intel_dp.c
> by the commit [71244653: drm/i915: adjusted_mode->clock in the dp
> mode_fix].
>
> In the past, the clock of the reference mode was modified in
> intel_dp_mode_fixup() in the case of eDP fixed clock, and this clock was
> used for calculating in intel_dp_set_m_n(). This override was removed,
> thus the wrong mode clock is used for the calculation, resulting in a
> psychedelic smoking output in the end.
>
> This patch corrects the clock to be used in the place.
>
> v1->v2: Use intel_edp_target_clock() for checking eDP fixed clock
> instead of open code as in ironlake_set_m_n().
>
> Cc: <[email protected]>
> Signed-off-by: Takashi Iwai <[email protected]>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 6f728e5..2606811 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -820,6 +820,7 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
> struct intel_link_m_n m_n;
> int pipe = intel_crtc->pipe;
> enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
> + int target_clock;
>
> /*
> * Find the lane count in the intel_encoder private
> @@ -835,13 +836,22 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
> }
> }
>
> + target_clock = mode->clock;
> + for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
> + if (intel_encoder->type == INTEL_OUTPUT_EDP) {
> + target_clock = intel_edp_target_clock(intel_encoder,
> + mode);
> + break;
> + }
> + }
> +
> /*
> * Compute the GMCH and Link ratios. The '3' here is
> * the number of bytes_per_pixel post-LUT, which we always
> * set up for 8-bits of R/G/B, or 3 bytes total.
> */
> intel_link_compute_m_n(intel_crtc->bpp, lane_count,
> - mode->clock, adjusted_mode->clock, &m_n);
> + target_clock, adjusted_mode->clock, &m_n);
>
> if (IS_HASWELL(dev)) {
> I915_WRITE(PIPE_DATA_M1(cpu_transcoder),
> --
> 1.8.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch