From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 06/11] drm/i915: Add intel_dotclock_calculate() Date: Fri, 13 Sep 2013 15:43:56 +0300 Message-ID: <20130913124356.GS20128@intel.com> References: <1378499348-4281-1-git-send-email-ville.syrjala@linux.intel.com> <1378499348-4281-7-git-send-email-ville.syrjala@linux.intel.com> <87k3iknbt0.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id D06D3E6627 for ; Fri, 13 Sep 2013 05:44:09 -0700 (PDT) Content-Disposition: inline In-Reply-To: <87k3iknbt0.fsf@intel.com> 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: Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Sep 13, 2013 at 03:30:51PM +0300, Jani Nikula wrote: > On Fri, 06 Sep 2013, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrj=E4l=E4 > > > > Extract the code to calculate the dotclock from the link clock and M/N > > values into a new function from ironlake_crtc_clock_get(). > > > > The new function can be used to calculate the dotclock for both FDI and > > DP cases. > > > > Also simplify the code a bit along the way. > > > > Signed-off-by: Ville Syrj=E4l=E4 > > --- > > drivers/gpu/drm/i915/intel_display.c | 40 +++++++++++++++-------------= -------- > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > 2 files changed, 19 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i91= 5/intel_display.c > > index b3049a6..c393c8e 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -7410,16 +7410,9 @@ static void i9xx_crtc_clock_get(struct intel_crt= c *crtc, > > pipe_config->adjusted_mode.clock =3D clock.dot; > > } > > = > > -static void ironlake_crtc_clock_get(struct intel_crtc *crtc, > > - struct intel_crtc_config *pipe_config) > > +int intel_dotclock_calculate(int link_freq, > > + const struct intel_link_m_n *m_n) > > { > > - struct drm_device *dev =3D crtc->base.dev; > > - struct drm_i915_private *dev_priv =3D dev->dev_private; > > - enum transcoder cpu_transcoder =3D pipe_config->cpu_transcoder; > > - int link_freq; > > - u64 clock; > > - u32 link_m, link_n; > > - > > /* > > * The calculation for the data clock is: > > * pixel_clock =3D ((m/n)*(link_clock * nr_lanes))/bpp > > @@ -7430,6 +7423,18 @@ static void ironlake_crtc_clock_get(struct intel= _crtc *crtc, > > * link_clock =3D (m * link_clock) / n > > */ > > = > > + if (!m_n->link_n) > > + return 0; > > + > > + return div_u64((u64)m_n->link_m * link_freq, m_n->link_n); > > +} > > + > > +static void ironlake_crtc_clock_get(struct intel_crtc *crtc, > > + struct intel_crtc_config *pipe_config) > > +{ > > + struct drm_device *dev =3D crtc->base.dev; > > + int link_freq; > > + > > /* > > * We need to get the FDI or DP link clock here to derive > > * the M/N dividers. > > @@ -7438,21 +7443,10 @@ static void ironlake_crtc_clock_get(struct inte= l_crtc *crtc, > > * For DP, it's either 1.62GHz or 2.7GHz. > > * We do our calculations in 10*MHz since we don't need much precison. > > */ > > - if (pipe_config->has_pch_encoder) > > - link_freq =3D intel_fdi_link_freq(dev) * 10000; > > - else > > - link_freq =3D pipe_config->port_clock; > = > The new code loses this distinction. I don't know if it matters. Please > explain. > = > > - > > - link_m =3D I915_READ(PIPE_LINK_M1(cpu_transcoder)); > > - link_n =3D I915_READ(PIPE_LINK_N1(cpu_transcoder)); > > - > > - if (!link_m || !link_n) > > - return; > > - > > - clock =3D ((u64)link_m * (u64)link_freq); > > - do_div(clock, link_n); > > + link_freq =3D intel_fdi_link_freq(dev) * 10000; > > = > > - pipe_config->adjusted_mode.clock =3D clock; > > + pipe_config->adjusted_mode.clock =3D > > + intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n); > = > I'm not sure if the fear in unwarranted, but can we always be sure > fdi_m_n has been set? I seem to have jumped the gun a bit with this patch. Patch 9 will make it so that we don't call ironlake_crtc_clock_get() except in the PCH encoder case (could use a rename of the function at that point maybe). Before patch 9 we still call this guy for non-PCH cases and then it might fall apart a bit. As the dp_m_n extraction patch appears before this patch in the series, I think I can just make a slightly fixed version of this patch. Stay tuned for v2... -- = Ville Syrj=E4l=E4 Intel OTC