Re: [PATCH 2/3] drm/i915: Generalize drain latency computation

Imre Deak
2014-07-31T13:34:23Z

[-- Attachment #1.1: Type: text/plain, Size: 5471 bytes --]

On Wed, 2014-07-16 at 18:24 +0530, Gajanan Bhat wrote:
> Modify drain latency computation to use it for any plane. Same function can be
> used for primary, cursor and sprite planes.
>
> Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 82 ++++++++++++++++++++++-----------------
> 2 files changed, 47 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d2a220b..a1260a2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3877,6 +3877,7 @@ enum punit_power_well {
> #define DDL_PLANE_PRECISION_64 (1<<7)
> #define DDL_PLANE_PRECISION_32 (0<<7)
> #define DDL_PLANE_SHIFT 0
> +#define DRAIN_LATENCY_MAX 0x7f

_MASK is the standard postfix in the driver.

>
> /* FIFO watermark sizes etc */
> #define G4X_FIFO_LINE_SIZE 64
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 90df1e8..f3a3e90 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -1268,33 +1268,21 @@ static bool g4x_compute_srwm(struct drm_device *dev,
> display, cursor);
> }
>
> -static bool vlv_compute_drain_latency(struct drm_device *dev,
> - int plane,
> - int *plane_prec_mult,
> - int *plane_dl,
> - int *cursor_prec_mult,
> - int *cursor_dl)
> +static bool vlv_compute_drain_latency(struct drm_crtc *crtc,
> + int pixel_size,
> + int *prec_mult,
> + int *drain_latency)
> {
> - struct drm_crtc *crtc;
> - int clock, pixel_size;
> int entries;
> + int clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
>
> - crtc = intel_get_crtc_for_plane(dev, plane);
> - if (!intel_crtc_active(crtc))
> + if (clock == 0 || pixel_size == 0)

This would mean a driver bug, so needs to be wrapped in a WARN().

> return false;
>
> - clock = to_intel_crtc(crtc)->config.adjusted_mode.crtc_clock;
> - pixel_size = crtc->primary->fb->bits_per_pixel / 8; /* BPP */
> -
> - entries = (clock / 1000) * pixel_size;
> - *plane_prec_mult = (entries > 128) ?
> - DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32;
> - *plane_dl = (64 * (*plane_prec_mult) * 4) / entries;
> -
> - entries = (clock / 1000) * 4; /* BPP is always 4 for cursor */
> - *cursor_prec_mult = (entries > 128) ?
> - DRAIN_LATENCY_PRECISION_64 : DRAIN_LATENCY_PRECISION_32;
> - *cursor_dl = (64 * (*cursor_prec_mult) * 4) / entries;
> + entries = DIV_ROUND_UP(clock, 1000) * pixel_size;
> + *prec_mult = (entries > 128) ? DRAIN_LATENCY_PRECISION_64 :
> + DRAIN_LATENCY_PRECISION_32;
> + *drain_latency = (64 * (*prec_mult) * 4) / entries;
>
> return true;
> }
> @@ -1309,24 +1297,46 @@ static bool vlv_compute_drain_latency(struct drm_device *dev,
>
> static void vlv_update_drain_latency(struct drm_crtc *crtc)
> {
> - struct drm_device *dev = crtc->dev;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> + int pixel_size;
> + int drain_latency;
> enum pipe pipe = to_intel_crtc(crtc)->pipe;
> - int plane_prec, plane_dl;
> - int cursor_prec, cursor_dl;
> - int plane_prec_mult, cursor_prec_mult;
> + int plane_prec, prec_mult, plane_dl;
>
> - if (vlv_compute_drain_latency(dev, pipe, &plane_prec_mult, &plane_dl,
> - &cursor_prec_mult, &cursor_dl)) {
> - cursor_prec = (cursor_prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> - DDL_CURSOR_PRECISION_64 : DDL_CURSOR_PRECISION_32;
> - plane_prec = (plane_prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> - DDL_PLANE_PRECISION_64 : DDL_PLANE_PRECISION_32;
> + plane_dl = I915_READ(VLV_DDL(pipe)) & ~DDL_PLANE_PRECISION_64 &
> + ~DRAIN_LATENCY_MAX & ~DDL_CURSOR_PRECISION_64 &
> + ~(DRAIN_LATENCY_MAX << DDL_CURSOR_SHIFT);

A tad simpler and more standard way for masking is ~(X | Y | Z).

> - I915_WRITE(VLV_DDL(pipe), cursor_prec |
> - (cursor_dl << DDL_CURSOR_SHIFT) |
> - plane_prec | (plane_dl << DDL_PLANE_SHIFT));
> + if (!intel_crtc_active(crtc)) {
> + I915_WRITE(VLV_DDL(pipe), plane_dl);
> + return;
> }
> +
> + /* Primary plane Drain Latency */
> + pixel_size = crtc->primary->fb->bits_per_pixel / 8; /* BPP */
> + if (vlv_compute_drain_latency(crtc, pixel_size, &prec_mult, &drain_latency)) {
> + plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> + DDL_PLANE_PRECISION_64 :
> + DDL_PLANE_PRECISION_32;
> + plane_dl = plane_dl | plane_prec | drain_latency;
> + }
> +
> + /* Cursor Drain Latency
> + * BPP is always 4 for cursor
> + */
> + pixel_size = 4;
> +
> + /* Program cursor DL only if it is enabled */
> + if ((I915_READ(CURCNTR(pipe)) & CURSOR_MODE) &&

Checking intel_crtc->cursor_base would be cheaper.

> + vlv_compute_drain_latency(crtc, pixel_size, &prec_mult, &drain_latency)) {
> + plane_prec = (prec_mult == DRAIN_LATENCY_PRECISION_64) ?
> + DDL_CURSOR_PRECISION_64 :
> + DDL_CURSOR_PRECISION_32;
> + plane_dl = plane_dl | plane_prec |
> + drain_latency << DDL_CURSOR_SHIFT;

This would overwrite the above assignment of plane_dl.

> + }
> +
> + I915_WRITE(VLV_DDL(pipe), plane_dl);
> }
>
> #define single_plane_enabled(mask) is_power_of_2(mask)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org