| Message ID | 1457945747-2161-7-git-send-email-ander.conselvan.de.oliveira@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 14-03-16 om 09:55 schreef Ander Conselvan de Oliveira: > Remove the clock calculation from ironlake_crtc_compute_clock() when the > encoder compute_config() already set one. The value was just thrown away > in that case. > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > It was thrown away, but it could still reject based on the limits, which this patch changes. This might be made more clear in the commit message.
On Mon, 2016-03-14 at 12:51 +0100, Maarten Lankhorst wrote: > Op 14-03-16 om 09:55 schreef Ander Conselvan de Oliveira: > > Remove the clock calculation from ironlake_crtc_compute_clock() when the > > encoder compute_config() already set one. The value was just thrown away > > in that case. > > > > Signed-off-by: Ander Conselvan de Oliveira < > > ander.conselvan.de.oliveira@intel.com> > > > It was thrown away, but it could still reject based on the limits, which this > patch changes. > This might be made more clear in the commit message. Good point. To be honest, I didn't very this as carefully as I should have before sending and missed that detail. It turns out that change is safe. To verify I extracted the relevant code and run it with all possible port clocks we could have with either the sdvo or the dp encoder setting the clock. See the attached C file. I was too lazy to actually understand what the g4x_find_best_dpll() does. Anyway, I'll send another version with a note about this. Thanks, Ander
Op 14-03-16 om 14:01 schreef Ander Conselvan De Oliveira: > On Mon, 2016-03-14 at 12:51 +0100, Maarten Lankhorst wrote: >> Op 14-03-16 om 09:55 schreef Ander Conselvan de Oliveira: >>> Remove the clock calculation from ironlake_crtc_compute_clock() when the >>> encoder compute_config() already set one. The value was just thrown away >>> in that case. >>> >>> Signed-off-by: Ander Conselvan de Oliveira < >>> ander.conselvan.de.oliveira@intel.com> >>> >> It was thrown away, but it could still reject based on the limits, which this >> patch changes. >> This might be made more clear in the commit message. > Good point. To be honest, I didn't very this as carefully as I should have > before sending and missed that detail. It turns out that change is safe. To > verify I extracted the relevant code and run it with all possible port clocks we > could have with either the sdvo or the dp encoder setting the clock. See the > attached C file. I was too lazy to actually understand what the > g4x_find_best_dpll() does. > I'm not sure this would have mattered even if the limits were different, since they wouldn't be used. Just something to make a note of. ~Maarten
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7034667..bf0416d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8889,7 +8889,7 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc, struct drm_device *dev = crtc->base.dev; intel_clock_t clock, reduced_clock; u32 dpll = 0, fp = 0, fp2 = 0; - bool ok, has_reduced_clock = false; + bool has_reduced_clock = false; bool is_lvds = false; struct intel_shared_dpll *pll; @@ -8901,14 +8901,15 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc, WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)), "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev)); - ok = ironlake_compute_clocks(&crtc->base, crtc_state, &clock, - &has_reduced_clock, &reduced_clock); - if (!ok && !crtc_state->clock_set) { - DRM_ERROR("Couldn't find PLL settings for mode!\n"); - return -EINVAL; - } - /* Compat-code for transition, will disappear. */ if (!crtc_state->clock_set) { + if (!ironlake_compute_clocks(&crtc->base, crtc_state, &clock, + &has_reduced_clock, + &reduced_clock)) { + DRM_ERROR("Couldn't find PLL settings for mode!\n"); + return -EINVAL; + } + + /* Compat-code for transition, will disappear. */ crtc_state->dpll.n = clock.n; crtc_state->dpll.m1 = clock.m1; crtc_state->dpll.m2 = clock.m2;
Remove the clock calculation from ironlake_crtc_compute_clock() when the encoder compute_config() already set one. The value was just thrown away in that case. Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)