From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 12/26] drm/i915: program drain latency regs on ValleyView Date: Sun, 25 Mar 2012 18:50:33 -0700 Message-ID: <20120326015032.GA6534@seagal.kumite> References: <1332452348-8814-1-git-send-email-jbarnes@virtuousgeek.org> <1332452348-8814-13-git-send-email-jbarnes@virtuousgeek.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id D4F409E826 for ; Sun, 25 Mar 2012 18:51:59 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1332452348-8814-13-git-send-email-jbarnes@virtuousgeek.org> 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: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Mar 22, 2012 at 02:38:54PM -0700, Jesse Barnes wrote: > From: Gajanan Bhat > > This patch adds support for programming drain latency registers of Pondicherry > memory arbiter of Valleyview. > > v2: clarify function names (Daniel) > fix summary typo (Daniel) > > Signed-off-by: Gajanan Bhat > Reviewed-by: Shobhit Kumar > Reviewed-by: Vijay Purushothaman > Reviewed-by: Jesse Barnes > Signed-off-by: Jesse Barnes > --- > drivers/gpu/drm/i915/i915_reg.h | 16 +++++++ > drivers/gpu/drm/i915/intel_display.c | 76 ++++++++++++++++++++++++++++++++++ > 2 files changed, 92 insertions(+), 0 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index be11d39..57e8980 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -2605,6 +2605,22 @@ > #define DSPFW_HPLL_CURSOR_MASK (0x3f<<16) > #define DSPFW_HPLL_SR_MASK (0x1ff) > > +/* drain latency register values*/ > +#define DRAIN_LATENCY_PRECISION_32 32 > +#define DRAIN_LATENCY_PRECISION_16 16 > +#define VLV_DDL1 0x70050 > +#define DDL_CURSORA_PRECISION_32 (1<<31) > +#define DDL_CURSORA_PRECISION_16 (0<<31) > +#define DDL_CURSORA_SHIFT 24 > +#define DDL_PLANEA_PRECISION_32 (1<<7) > +#define DDL_PLANEA_PRECISION_16 (0<<7) > +#define VLV_DDL2 0x70054 > +#define DDL_CURSORB_PRECISION_32 (1<<31) > +#define DDL_CURSORB_PRECISION_16 (0<<31) > +#define DDL_CURSORB_SHIFT 24 > +#define DDL_PLANEB_PRECISION_32 (1<<7) > +#define DDL_PLANEB_PRECISION_16 (0<<7) > + > /* FIFO watermark sizes etc */ > #define G4X_FIFO_LINE_SIZE 64 > #define I915_FIFO_LINE_SIZE 64 > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 6bed629..4b5905c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4337,6 +4337,80 @@ static bool g4x_compute_srwm(struct drm_device *dev, > display, cursor); > } > > +static int vlv_compute_drain_latency(struct drm_device *dev, > + int plane, > + int *plane_prec_mult, > + int *plane_dl, > + int *cursor_prec_mult, > + int *cursor_dl) > +{ > + struct drm_crtc *crtc; > + int clock, pixel_size; > + int entries; > + > + crtc = intel_get_crtc_for_plane(dev, plane); > + if (crtc->fb == NULL || !crtc->enabled) > + return false; > + > + clock = crtc->mode.clock; /* VESA DOT Clock */ > + pixel_size = crtc->fb->bits_per_pixel / 8; /* BPP */ > + > + entries = (clock / 1000) * pixel_size; > + *plane_prec_mult = (entries > 256) ? > + DRAIN_LATENCY_PRECISION_32 : DRAIN_LATENCY_PRECISION_16; > + *plane_dl = (64 * *plane_prec_mult * 4) / ((clock / 1000) * pixel_size); > + > + entries = (clock / 1000) * 4; /* BPP is always 4 for cursor */ > + *cursor_prec_mult = (entries > 256) ? > + DRAIN_LATENCY_PRECISION_32 : DRAIN_LATENCY_PRECISION_16; > + *cursor_dl = (64 * *cursor_prec_mult * 4) / ((clock / 1000) * 4); Can we add some parentheses around *cursor_prec_mult here (two places above)? > + > + return true; > +} It doesn't produce a warning for me, but 0, and 1 are more typically returned when using ints. I'd just change the return type to bool. > + > +/* > + * Update drain latency registers of memory arbiter > + * > + * Valleyview SoC has a new memory arbiter and needs drain latency registers > + * to be programmed. Each plane has a drain latency multiplier and a drain > + * latency value. > + */ Better than this comment would have been how the arbiter's drain latency functions. > + > +static void vlv_update_drain_latency(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + int planea_prec, planea_dl, planeb_prec, planeb_dl; > + int cursora_prec, cursora_dl, cursorb_prec, cursorb_dl; > + int plane_prec_mult, cursor_prec_mult; /* Precision multiplier is > + either 16 or 32 */ > + > + /* For plane A, Cursor A */ > + if (vlv_compute_drain_latency(dev, 0, &plane_prec_mult, &planea_dl, > + &cursor_prec_mult, &cursora_dl)) { > + cursora_prec = (cursor_prec_mult == DRAIN_LATENCY_PRECISION_32) ? > + DDL_CURSORA_PRECISION_32 : DDL_CURSORA_PRECISION_16; > + planea_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_32) ? > + DDL_PLANEA_PRECISION_32 : DDL_PLANEA_PRECISION_16; > + > + I915_WRITE(VLV_DDL1, cursora_prec | > + (cursora_dl << DDL_CURSORA_SHIFT) | > + planea_prec | planea_dl); > + } > + > + /* For plane B, Cursor B */ > + if (vlv_compute_drain_latency(dev, 1, &plane_prec_mult, &planeb_dl, > + &cursor_prec_mult, &cursorb_dl)) { > + cursorb_prec = (cursor_prec_mult == DRAIN_LATENCY_PRECISION_32) ? > + DDL_CURSORB_PRECISION_32 : DDL_CURSORB_PRECISION_16; > + planeb_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_32) ? > + DDL_PLANEB_PRECISION_32 : DDL_PLANEB_PRECISION_16; > + > + I915_WRITE(VLV_DDL2, cursorb_prec | > + (cursorb_dl << DDL_CURSORB_SHIFT) | > + planeb_prec | planeb_dl); > + } > +} > + > #define single_plane_enabled(mask) is_power_of_2(mask) > > static void valleyview_update_wm(struct drm_device *dev) > @@ -4347,6 +4421,8 @@ static void valleyview_update_wm(struct drm_device *dev) > int plane_sr, cursor_sr; > unsigned int enabled = 0; > > + vlv_update_drain_latency(dev); > + > if (g4x_compute_wm0(dev, 0, > &valleyview_wm_info, latency_ns, > &valleyview_cursor_wm_info, latency_ns, Couldn't find the registers in docs, so only Acked-by: Ben Widawsky