On Mon, May 08, 2017 at 05:18:55PM +0530, Mahesh Kumar wrote:
> This patch make changes to calculate adjusted plane pixel rate &
> plane downscale amount using fixed_point functions available.
> This patch will give uniformity in code, & will help to avoid mixing of
> 32bit uint32_t variable for fixed-16.16 with fixed_16_16_t variables in
> later patch in the series.
>
> Signed-off-by: Mahesh Kumar <[email protected]>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 37 +++++++++++++++++++------------------
> 1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 66b542ba47ad..6414701ef09e 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3360,26 +3360,27 @@ void skl_ddb_get_hw_state(struct drm_i915_private
> *dev_priv,
> * Return value is provided in 16.16 fixed point form to retain fractional
> part.
> * Caller should take care of dividing & rounding off the value.
> */
> -static uint32_t
> +static uint_fixed_16_16_t
> skl_plane_downscale_amount(const struct intel_crtc_state *cstate,
> const struct intel_plane_state *pstate)
> {
> struct intel_plane *plane = to_intel_plane(pstate->base.plane);
> - uint32_t downscale_h, downscale_w;
> uint32_t src_w, src_h, dst_w, dst_h;
> + uint_fixed_16_16_t fp_w_ratio, fp_h_ratio;
> + uint_fixed_16_16_t downscale_h, downscale_w;
>
> if (WARN_ON(!intel_wm_plane_visible(cstate, pstate)))
> - return DRM_PLANE_HELPER_NO_SCALING;
> + return u32_to_fixed_16_16(0);
I don't think this really changes anything, right? Both of the places
that call this function have already bailed out and returned 0 data rate
if the plane is invisible. Not that it hurts anything either.
>
> /* n.b., src is 16.16 fixed point, dst is whole integer */
> if (plane->id == PLANE_CURSOR) {
> - src_w = pstate->base.src_w;
> - src_h = pstate->base.src_h;
> + src_w = pstate->base.src_w >> 16;
> + src_h = pstate->base.src_h >> 16;
> dst_w = pstate->base.crtc_w;
> dst_h = pstate->base.crtc_h;
> } else {
> - src_w = drm_rect_width(&pstate->base.src);
> - src_h = drm_rect_height(&pstate->base.src);
> + src_w = drm_rect_width(&pstate->base.src) >> 16;
> + src_h = drm_rect_height(&pstate->base.src) >> 16;
> dst_w = drm_rect_width(&pstate->base.dst);
> dst_h = drm_rect_height(&pstate->base.dst);
> }
> @@ -3387,11 +3388,13 @@ skl_plane_downscale_amount(const struct
> intel_crtc_state *cstate,
> if (drm_rotation_90_or_270(pstate->base.rotation))
> swap(dst_w, dst_h);
>
> - downscale_h = max(src_h / dst_h, (uint32_t)DRM_PLANE_HELPER_NO_SCALING);
> - downscale_w = max(src_w / dst_w, (uint32_t)DRM_PLANE_HELPER_NO_SCALING);
> + fp_w_ratio = fixed_16_16_div(src_w, dst_w);
> + fp_h_ratio = fixed_16_16_div(src_h, dst_h);
> + downscale_w = max_fixed_16_16(fp_w_ratio, u32_to_fixed_16_16(1));
> + downscale_h = max_fixed_16_16(fp_h_ratio, u32_to_fixed_16_16(1));
>
> /* Provide result in 16.16 fixed point */
> - return (uint64_t)downscale_w * downscale_h >> 16;
> + return mul_fixed_16_16(downscale_w, downscale_h);
I think we can drop the comment here now; it was originally added to
remind why we were doing the extra shift at return, but now it's pretty
obvious at this point that everything here is dealing with fixed point.
With or without the comment dropped,
Reviewed-by: Matt Roper <[email protected]>
> }
>
> static unsigned int
> @@ -3401,10 +3404,11 @@ skl_plane_relative_data_rate(const struct
> intel_crtc_state *cstate,
> {
> struct intel_plane *plane = to_intel_plane(pstate->plane);
> struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
> - uint32_t down_scale_amount, data_rate;
> + uint32_t data_rate;
> uint32_t width = 0, height = 0;
> struct drm_framebuffer *fb;
> u32 format;
> + uint_fixed_16_16_t down_scale_amount;
>
> if (!intel_pstate->base.visible)
> return 0;
> @@ -3438,7 +3442,7 @@ skl_plane_relative_data_rate(const struct
> intel_crtc_state *cstate,
>
> down_scale_amount = skl_plane_downscale_amount(cstate, intel_pstate);
>
> - return (uint64_t)data_rate * down_scale_amount >> 16;
> + return mul_u32_fixed_16_16_round_up(data_rate, down_scale_amount);
> }
>
> /*
> @@ -3724,8 +3728,7 @@ static uint32_t skl_adjusted_plane_pixel_rate(const
> struct intel_crtc_state *cst
> struct intel_plane_state *pstate)
> {
> uint64_t adjusted_pixel_rate;
> - uint64_t downscale_amount;
> - uint64_t pixel_rate;
> + uint_fixed_16_16_t downscale_amount;
>
> /* Shouldn't reach here on disabled planes... */
> if (WARN_ON(!intel_wm_plane_visible(cstate, pstate)))
> @@ -3738,10 +3741,8 @@ static uint32_t skl_adjusted_plane_pixel_rate(const
> struct intel_crtc_state *cst
> adjusted_pixel_rate = cstate->pixel_rate;
> downscale_amount = skl_plane_downscale_amount(cstate, pstate);
>
> - pixel_rate = adjusted_pixel_rate * downscale_amount >> 16;
> - WARN_ON(pixel_rate != clamp_t(uint32_t, pixel_rate, 0, ~0));
> -
> - return pixel_rate;
> + return mul_u32_fixed_16_16_round_up(adjusted_pixel_rate,
> + downscale_amount);
> }
>
> static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> --
> 2.11.0
>
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
