From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH] drm/i915: compute the target_clock for edp directly Date: Thu, 31 May 2012 11:31:50 +0100 Message-ID: <1338460346_378547@CP5-2952> References: <20120530125139.GG6888@phenom.ffwll.local> <1338384860-1495-1-git-send-email-daniel.vetter@ffwll.ch> <1338457831_377831@CP5-2952> <20120531100353.GA664@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from fireflyinternet.com (smtp.fireflyinternet.com [109.228.6.236]) by gabe.freedesktop.org (Postfix) with ESMTP id 5B37BA097B for ; Thu, 31 May 2012 03:32:28 -0700 (PDT) In-Reply-To: <20120531100353.GA664@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Thu, 31 May 2012 12:03:53 +0200, Daniel Vetter wrote: > On Thu, May 31, 2012 at 10:49:59AM +0100, Chris Wilson wrote: > > On Wed, 30 May 2012 15:34:20 +0200, Daniel Vetter wrote: > > > Instead of abusing into mode->clock, because we should touch that > > > one at all. First prep step to constify the mode argument to the > > > intel_dp_mode_fixup function. > > > > > > The next patch will stop us from modifying mode->clock. > > > > > > Signed-Off-by: Daniel Vetter > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 16 ++++++++-------- > > > drivers/gpu/drm/i915/intel_dp.c | 12 ++++++++++++ > > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > > 3 files changed, 22 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index 9147894..a5d06ed 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -4431,16 +4431,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > > > /* CPU eDP doesn't require FDI link, so just set DP M/N > > > according to current link config */ > > > if (is_cpu_edp) { > > > - target_clock = mode->clock; > > > intel_edp_link_config(edp_encoder, &lane, &link_bw); > > > } else { > > > - /* [e]DP over FDI requires target mode clock > > > - instead of link clock */ > > > - if (is_dp) > > > - target_clock = mode->clock; > > > - else > > > - target_clock = adjusted_mode->clock; > > > - > > > /* FDI is a binary signal running at ~2.7GHz, encoding > > > * each output octet as 10 bits. The actual frequency > > > * is stored as a divider into a 100MHz clock, and the > > > @@ -4451,6 +4443,14 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > > > link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10; > > > } > > > > > > + /* [e]DP over FDI requires target mode clock instead of link clock. */ > > > + if (edp_encoder) > > > + target_clock = intel_edp_target_clock(edp_encoder, mode); > > > > Given the edp_encoder, we know that adjusted_mode contains either the > > fixed_panel clock or the original clock depending on whether a fixed > > mode was found. So the layering violation could be dropped here in > > favour of target_clock = adjusted_mode->clok. > > It's not that simple. Assuming I understand this maze correctly, we're > dealing with 3 different clocks. > - The dp link clock, both the old and new code store that in > adjusted_mode->clock at the end of intel_dp_mode_fixup. Right, missed that adjusted_mode->clock was rewritten in dp_mode_fixup. In fact, wouldn't that make the better export from intel_dp? target_clock = intel_dp_link_clock(adjusted_mode); with a judicious adjusted_mode->private_flags |= INTEL_DP_LINK_BW_2_7/1_8? -Chris -- Chris Wilson, Intel Open Source Technology Centre