From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vandana Kannan Subject: Re: [PATCH 1/2] drm/i915: Get CZ clock for VLV Date: Tue, 05 Aug 2014 20:57:07 +0530 Message-ID: <53E0F7CB.509@intel.com> References: <1407172445-5730-1-git-send-email-vandana.kannan@intel.com> <20140805130958.GL4193@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 E49F26E190 for ; Tue, 5 Aug 2014 08:27:10 -0700 (PDT) In-Reply-To: <20140805130958.GL4193@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: =?ISO-8859-1?Q?Ville_Syrj=E4l=E4?= Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Aug-05-2014 6:39 PM, Ville Syrj=E4l=E4 wrote: > On Mon, Aug 04, 2014 at 10:44:04PM +0530, Vandana Kannan wrote: >> CZ clock is related to data flow from memory to display plane. This is >> required for comparison with CD clock before programming PFI credits. >> >> Signed-off-by: Vandana Kannan >> --- >> drivers/gpu/drm/i915/i915_drv.h | 2 ++ >> drivers/gpu/drm/i915/i915_reg.h | 1 + >> drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++++++ >> 3 files changed, 23 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915= _drv.h >> index 174a294..baeb56f 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -2764,6 +2764,8 @@ void vlv_flisdsi_write(struct drm_i915_private *de= v_priv, u32 reg, u32 val); >> int vlv_gpu_freq(struct drm_i915_private *dev_priv, int val); >> int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val); >> = >> +int valleyview_get_cz_clock_speed(struct drm_device *dev); >> + >> #define FORCEWAKE_RENDER (1 << 0) >> #define FORCEWAKE_MEDIA (1 << 1) >> #define FORCEWAKE_ALL (FORCEWAKE_RENDER | FORCEWAKE_MEDIA) >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915= _reg.h >> index fe5c276..1b8f095 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -601,6 +601,7 @@ enum punit_power_well { >> #define DISPLAY_FREQUENCY_STATUS (0x1f << 8) >> #define DISPLAY_FREQUENCY_STATUS_SHIFT 8 >> #define DISPLAY_FREQUENCY_VALUES (0x1f << 0) >> +#define CCK_CZ_CONTROL 0x62 > = > Please move the reg define to just above DISPLAY_CLOCK_CONTROL to keep > things in neat order. > = Ok, will make this change.. > Also it might be a good idea to rename the DISPLAY_TRUNK_* and > DISPLAY_FREQUENCY_* bits to CCK_... instead of DISPLAY_... to make > it clear they apply to all CCK clock control registers. > = Will submit this renaming part as a separate patch.. >> = >> /** >> * DOC: DPIO >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915= /intel_display.c >> index 99eb7ca..56a8090 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -5278,6 +5278,26 @@ static int valleyview_get_display_clock_speed(str= uct drm_device *dev) >> return DIV_ROUND_CLOSEST(vco << 1, divider + 1); >> } >> = >> +int valleyview_get_cz_clock_speed(struct drm_device *dev) >> +{ >> + struct drm_i915_private *dev_priv =3D dev->dev_private; >> + int vco =3D valleyview_get_vco(dev_priv); >> + u32 val; >> + int divider; >> + >> + mutex_lock(&dev_priv->dpio_lock); >> + val =3D vlv_cck_read(dev_priv, CCK_CZ_CONTROL); >> + mutex_unlock(&dev_priv->dpio_lock); >> + >> + divider =3D val & DISPLAY_FREQUENCY_VALUES; >> + >> + WARN((val & DISPLAY_FREQUENCY_STATUS) !=3D >> + (divider << DISPLAY_FREQUENCY_STATUS_SHIFT), >> + "czclk change in progress\n"); >> + >> + return DIV_ROUND_CLOSEST(vco << 1, divider + 1); >> +} > = > We might want to refactor this a bit. Eg.: > = > static int valleyview_get_cck_clock_speed(u32 reg, const char *name) > { > ... do the cck read etc. from CCK clock control register 'reg' > } > = > static int valleyview_get_cz_clock_speed() > { > return valleyview_get_cck_clock_speed(CCK_CZ_CLOCK_CONTROL, "czclk"); > } > = > static int valleyview_get_display_clock_speed() > { > ... > return valleyview_get_cck_clock_speed(CCK_DISPLAY_CLOCK_CONTROL, "cdclk"= ); > } > = As part of the above refactoring, how about having an enum to specify CDCLK or CZCLK instead of char *? - Vandana >> + >> static int i945_get_display_clock_speed(struct drm_device *dev) >> { >> return 400000; >> -- = >> 2.0.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > =