[Intel-gfx] [v2 3/6] drm/i915/skl+: calculate plane pixel rate
Matt Roper
matthew.d.roper at intel.com
Wed Feb 10 18:53:48 UTC 2016
On Wed, Jan 27, 2016 at 09:40:00PM +0530, Shobhit Kumar wrote:
> From: "Kumar, Mahesh" <mahesh1.kumar at intel.com>
>
> Don't use pipe pixel rate for plane pixel rate. Calculate plane pixel according
> to formula
>
> adjusted plane_pixel_rate = adjusted pipe_pixel_rate * downscale ammount
>
> downscale amount = max[1, src_h/dst_h] * max[1, src_w/dst_w]
> if 90/270 rotation use rotated width & height
>
> v2: use intel_plane_state->visible instead of (fb == NULL) as per Matt's
> comment.
>
> Cc: matthew.d.roper at intel.com
> Signed-off-by: Kumar, Mahesh <mahesh1.kumar at intel.com>
> ---
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> drivers/gpu/drm/i915/intel_pm.c | 88 +++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 88 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bc97012..bb2b1c7 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -638,6 +638,8 @@ struct intel_plane_wm_parameters {
> u64 tiling;
> unsigned int rotation;
> uint16_t fifo_size;
> + /* Stores the adjusted plane pixel rate for WM calculation for SKL+ */
> + uint32_t plane_pixel_rate;
> };
>
> struct intel_plane {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 708f329..40fff09 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2782,6 +2782,48 @@ skl_wm_plane_id(const struct intel_plane *plane)
> }
> }
>
> +/*
> + * This function takes drm_plane_state as input
> + * and decides the downscale amount according to the formula
> + *
> + * downscale amount = Max[1, Horizontal source size / Horizontal dest size]
> + *
> + * Return value is multiplied by 1000 to retain fractional part
> + * Caller should take care of dividing & Rounding off the value
> + */
> +static uint32_t
> +skl_plane_downscale_amount(const struct intel_plane *intel_plane)
> +{
> + struct drm_plane_state *pstate = intel_plane->base.state;
> + struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
> + uint32_t downscale_h, downscale_w;
> + uint32_t src_w, src_h, dst_w, dst_h, tmp;
> +
> + /* If plane not visible return amount as unity */
> + if (!intel_pstate->visible)
> + return 1000;
> +
> + src_w = drm_rect_width(&intel_pstate->src) >> 16;
> + src_h = drm_rect_height(&intel_pstate->src) >> 16;
> +
> + dst_w = drm_rect_width(&intel_pstate->dst);
> + dst_h = drm_rect_height(&intel_pstate->dst);
> +
> + if (intel_rotation_90_or_270(pstate->rotation))
> + swap(dst_w, dst_h);
> +
> + /* Multiply by 1000 for precision */
> + tmp = (1000 * src_h) / dst_h;
> + downscale_h = max_t(uint32_t, 1000, tmp);
> +
> + tmp = (1000 * src_w) / dst_w;
> + downscale_w = max_t(uint32_t, 1000, tmp);
> +
> + /* Reducing precision to 3 decimal places */
> + return DIV_ROUND_UP(downscale_h * downscale_w, 1000);
> +}
I think I mentioned it on my earlier review, but it feels like it would
be simpler/more consistent to just continue using 16.16 binary fixed
point instead of switching over to decimal fixed point (and maybe call
drm_rect_calc_[hv]scale to calculate the scaling). Is there a specific
need to switch?
> +
> +
> static void
> skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
> const struct intel_crtc_state *cstate,
> @@ -3183,10 +3225,10 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> swap(width, height);
>
> bytes_per_pixel = drm_format_plane_cpp(fb->pixel_format, 0);
> - method1 = skl_wm_method1(skl_pipe_pixel_rate(cstate),
> + method1 = skl_wm_method1(intel_plane->wm.plane_pixel_rate,
> bytes_per_pixel,
> latency);
> - method2 = skl_wm_method2(skl_pipe_pixel_rate(cstate),
> + method2 = skl_wm_method2(intel_plane->wm.plane_pixel_rate,
> cstate->base.adjusted_mode.crtc_htotal,
> width,
> bytes_per_pixel,
> @@ -3627,6 +3669,45 @@ static void skl_update_other_pipe_wm(struct drm_device *dev,
> }
> }
>
> +static uint32_t
> +skl_plane_pixel_rate(struct intel_crtc_state *cstate, struct intel_plane *plane)
> +{
> + uint32_t adjusted_pixel_rate;
> + uint32_t downscale_amount;
> +
> + /*
> + * adjusted plane pixel rate = adjusted pipe pixel rate
> + * Plane pixel rate = adjusted plane pixel rate * plane down scale
> + * amount
> + */
> + adjusted_pixel_rate = skl_pipe_pixel_rate(cstate);
> + downscale_amount = skl_plane_downscale_amount(plane);
> +
> + return DIV_ROUND_UP(adjusted_pixel_rate * downscale_amount,
> + 1000);
> +}
> +
> +static void skl_set_plane_pixel_rate(struct drm_crtc *crtc)
> +{
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> + struct intel_plane *intel_plane = NULL;
> + struct drm_device *dev = crtc->dev;
> +
> + if (!intel_crtc->active)
> + return;
> + for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> + struct drm_plane *plane = &intel_plane->base;
> +
> + if (!to_intel_plane_state(plane->state)->visible)
> + continue;
> +
> + intel_plane->wm.plane_pixel_rate = skl_plane_pixel_rate(cstate,
> + intel_plane);
Can we move this to intel_plane_state instead? We've eliminated all
usage of intel_plane->wm from ILK and SKL watermarks, so that structure
is only used on VLV today.
Matt
> + }
> +
> +}
> +
> static void skl_clear_wm(struct skl_wm_values *watermarks, enum pipe pipe)
> {
> watermarks->wm_linetime[pipe] = 0;
> @@ -3662,6 +3743,9 @@ static void skl_update_wm(struct drm_crtc *crtc)
>
> skl_clear_wm(results, intel_crtc->pipe);
>
> + /* Calculate plane pixel rate for each plane in advance */
> + skl_set_plane_pixel_rate(crtc);
> +
> if (!skl_update_pipe_wm(crtc, &results->ddb, pipe_wm))
> return;
>
> --
> 2.5.0
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
More information about the Intel-gfx
mailing list