[Intel-gfx] [PATCH 02/15] drm/i915/skl+: refactore WM calculation for NV12
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Thu Jan 4 16:19:33 UTC 2018
Op 07-01-18 om 10:59 schreef Vidya Srinivas:
> From: Mahesh Kumar <mahesh1.kumar at intel.com>
>
> Current code calculates DDB for planar formats in such a way that we
> store DDB of plane-0 in plane 1 & vice-versa.
> In order to make this clean this patch refactors WM/DDB calculation for
> NV12 planar formats.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar at intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 120 +++++++++++++++++++--------------------
> 3 files changed, 62 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 56047f8..962717d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1434,7 +1434,7 @@ static inline bool skl_ddb_entry_equal(const struct skl_ddb_entry *e1,
>
> struct skl_ddb_allocation {
> struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; /* packed/uv */
^Should this be changed to packed/y?
> - struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
> + struct skl_ddb_entry uv_plane[I915_MAX_PIPES][I915_MAX_PLANES];
> };
>
> struct skl_ddb_values {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 9c8aef9..5f5e070 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -513,6 +513,7 @@ struct intel_pipe_wm {
> struct skl_plane_wm {
> struct skl_wm_level wm[8];
> struct skl_wm_level trans_wm;
> + bool is_nv12;
> };
>
> struct skl_pipe_wm {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e2598cf..15edb9a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4004,9 +4004,9 @@ int skl_check_pipe_max_pixel_rate(struct intel_crtc *intel_crtc,
> static unsigned int
> skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> const struct drm_plane_state *pstate,
> - int y)
> + const int plane)
> {
> - struct intel_plane *plane = to_intel_plane(pstate->plane);
> + struct intel_plane *intel_plane = to_intel_plane(pstate->plane);
> struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
> uint32_t data_rate;
> uint32_t width = 0, height = 0;
> @@ -4020,9 +4020,9 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> fb = pstate->fb;
> format = fb->format->format;
>
> - if (plane->id == PLANE_CURSOR)
> + if (intel_plane->id == PLANE_CURSOR)
> return 0;
> - if (y && format != DRM_FORMAT_NV12)
> + if (plane == 1 && format != DRM_FORMAT_NV12)
> return 0;
>
> /*
> @@ -4033,19 +4033,14 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> width = drm_rect_width(&intel_pstate->base.src) >> 16;
> height = drm_rect_height(&intel_pstate->base.src) >> 16;
>
> - /* for planar format */
> - if (format == DRM_FORMAT_NV12) {
> - if (y) /* y-plane data rate */
> - data_rate = width * height *
> - fb->format->cpp[0];
> - else /* uv-plane data rate */
> - data_rate = (width / 2) * (height / 2) *
> - fb->format->cpp[1];
> - } else {
> - /* for packed formats */
> - data_rate = width * height * fb->format->cpp[0];
> + /* UV plane does 1/2 pixel sub-sampling */
> + if (plane == 1 && format == DRM_FORMAT_NV12) {
> + width /= 2;
> + height /= 2;
> }
>
> + data_rate = width * height * fb->format->cpp[plane];
> +
> down_scale_amount = skl_plane_downscale_amount(cstate, intel_pstate);
>
> return mul_round_up_u32_fixed16(data_rate, down_scale_amount);
> @@ -4058,8 +4053,8 @@ skl_plane_relative_data_rate(const struct intel_crtc_state *cstate,
> */
> static unsigned int
> skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
> - unsigned *plane_data_rate,
> - unsigned *plane_y_data_rate)
> + unsigned int *plane_data_rate,
> + unsigned int *uv_plane_data_rate)
> {
> struct drm_crtc_state *cstate = &intel_cstate->base;
> struct drm_atomic_state *state = cstate->state;
> @@ -4075,17 +4070,16 @@ skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
> enum plane_id plane_id = to_intel_plane(plane)->id;
> unsigned int rate;
>
> - /* packed/uv */
packed/y?
> rate = skl_plane_relative_data_rate(intel_cstate,
> pstate, 0);
> plane_data_rate[plane_id] = rate;
>
> total_data_rate += rate;
>
> - /* y-plane */
> + /* uv-plane */
> rate = skl_plane_relative_data_rate(intel_cstate,
> pstate, 1);
> - plane_y_data_rate[plane_id] = rate;
> + uv_plane_data_rate[plane_id] = rate;
>
> total_data_rate += rate;
> }
> @@ -4094,8 +4088,7 @@ skl_get_total_relative_data_rate(struct intel_crtc_state *intel_cstate,
> }
>
> static uint16_t
> -skl_ddb_min_alloc(const struct drm_plane_state *pstate,
> - const int y)
> +skl_ddb_min_alloc(const struct drm_plane_state *pstate, const int plane)
> {
> struct drm_framebuffer *fb = pstate->fb;
> struct intel_plane_state *intel_pstate = to_intel_plane_state(pstate);
> @@ -4106,8 +4099,8 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
> if (WARN_ON(!fb))
> return 0;
>
> - /* For packed formats, no y-plane, return 0 */
> - if (y && fb->format->format != DRM_FORMAT_NV12)
> + /* For packed formats, and uv-plane, return 0 */
> + if (plane == 1 && fb->format->format != DRM_FORMAT_NV12)
> return 0;
>
> /* For Non Y-tile return 8-blocks */
> @@ -4126,15 +4119,12 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
> src_h = drm_rect_height(&intel_pstate->base.src) >> 16;
>
> /* Halve UV plane width and height for NV12 */
> - if (fb->format->format == DRM_FORMAT_NV12 && !y) {
> + if (plane == 1) {
> src_w /= 2;
> src_h /= 2;
> }
>
> - if (fb->format->format == DRM_FORMAT_NV12 && !y)
> - plane_bpp = fb->format->cpp[1];
> - else
> - plane_bpp = fb->format->cpp[0];
> + plane_bpp = fb->format->cpp[plane];
>
> if (drm_rotation_90_or_270(pstate->rotation)) {
> switch (plane_bpp) {
> @@ -4162,7 +4152,7 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate,
>
> static void
> skl_ddb_calc_min(const struct intel_crtc_state *cstate, int num_active,
> - uint16_t *minimum, uint16_t *y_minimum)
> + uint16_t *minimum, uint16_t *uv_minimum)
> {
> const struct drm_plane_state *pstate;
> struct drm_plane *plane;
> @@ -4177,7 +4167,7 @@ skl_ddb_calc_min(const struct intel_crtc_state *cstate, int num_active,
> continue;
>
> minimum[plane_id] = skl_ddb_min_alloc(pstate, 0);
> - y_minimum[plane_id] = skl_ddb_min_alloc(pstate, 1);
> + uv_minimum[plane_id] = skl_ddb_min_alloc(pstate, 1);
> }
>
> minimum[PLANE_CURSOR] = skl_cursor_allocation(num_active);
> @@ -4195,17 +4185,17 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb;
> uint16_t alloc_size, start;
> uint16_t minimum[I915_MAX_PLANES] = {};
> - uint16_t y_minimum[I915_MAX_PLANES] = {};
> + uint16_t uv_minimum[I915_MAX_PLANES] = {};
> unsigned int total_data_rate;
> enum plane_id plane_id;
> int num_active;
> - unsigned plane_data_rate[I915_MAX_PLANES] = {};
> - unsigned plane_y_data_rate[I915_MAX_PLANES] = {};
> + unsigned int plane_data_rate[I915_MAX_PLANES] = {};
> + unsigned int uv_plane_data_rate[I915_MAX_PLANES] = {};
> uint16_t total_min_blocks = 0;
>
> /* Clear the partitioning for disabled planes. */
> memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
> - memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
> + memset(ddb->uv_plane[pipe], 0, sizeof(ddb->uv_plane[pipe]));
>
> if (WARN_ON(!state))
> return 0;
> @@ -4220,7 +4210,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> if (alloc_size == 0)
> return 0;
>
> - skl_ddb_calc_min(cstate, num_active, minimum, y_minimum);
> + skl_ddb_calc_min(cstate, num_active, minimum, uv_minimum);
>
> /*
> * 1. Allocate the mininum required blocks for each active plane
> @@ -4230,7 +4220,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>
> for_each_plane_id_on_crtc(intel_crtc, plane_id) {
> total_min_blocks += minimum[plane_id];
> - total_min_blocks += y_minimum[plane_id];
> + total_min_blocks += uv_minimum[plane_id];
> }
>
> if (total_min_blocks > alloc_size) {
> @@ -4252,14 +4242,14 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> */
> total_data_rate = skl_get_total_relative_data_rate(cstate,
> plane_data_rate,
> - plane_y_data_rate);
> + uv_plane_data_rate);
> if (total_data_rate == 0)
> return 0;
>
> start = alloc->start;
> for_each_plane_id_on_crtc(intel_crtc, plane_id) {
> - unsigned int data_rate, y_data_rate;
> - uint16_t plane_blocks, y_plane_blocks = 0;
> + unsigned int data_rate, uv_data_rate;
> + uint16_t plane_blocks, uv_plane_blocks;
>
> if (plane_id == PLANE_CURSOR)
> continue;
> @@ -4283,21 +4273,20 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>
> start += plane_blocks;
>
> - /*
> - * allocation for y_plane part of planar format:
> - */
> - y_data_rate = plane_y_data_rate[plane_id];
> + /* Allocate DDB for UV plane for planar format/NV12 */
> + uv_data_rate = uv_plane_data_rate[plane_id];
>
> - y_plane_blocks = y_minimum[plane_id];
> - y_plane_blocks += div_u64((uint64_t)alloc_size * y_data_rate,
> + uv_plane_blocks = uv_minimum[plane_id];
> + uv_plane_blocks += div_u64((uint64_t)alloc_size * uv_data_rate,
> total_data_rate);
>
> - if (y_data_rate) {
> - ddb->y_plane[pipe][plane_id].start = start;
> - ddb->y_plane[pipe][plane_id].end = start + y_plane_blocks;
> + if (uv_data_rate) {
> + ddb->uv_plane[pipe][plane_id].start = start;
> + ddb->uv_plane[pipe][plane_id].end = start +
> + uv_plane_blocks;
> }
>
> - start += y_plane_blocks;
> + start += uv_plane_blocks;
> }
>
> return 0;
> @@ -4425,8 +4414,7 @@ skl_compute_plane_wm_params(const struct drm_i915_private *dev_priv,
> wp->width = drm_rect_width(&intel_pstate->base.src) >> 16;
> }
>
> - wp->cpp = (fb->format->format == DRM_FORMAT_NV12) ? fb->format->cpp[1] :
> - fb->format->cpp[0];
> + wp->cpp = fb->format->cpp[0];
> wp->plane_pixel_rate = skl_adjusted_plane_pixel_rate(cstate,
> intel_pstate);
>
> @@ -4618,6 +4606,9 @@ skl_compute_wm_levels(const struct drm_i915_private *dev_priv,
> return ret;
> }
>
> + if (intel_pstate->base.fb->format->format == DRM_FORMAT_NV12)
> + wm->is_nv12 = true;
> +
> return 0;
> }
>
> @@ -4788,10 +4779,19 @@ static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
> skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
> &wm->trans_wm);
>
> - skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
> - &ddb->plane[pipe][plane_id]);
> - skl_ddb_entry_write(dev_priv, PLANE_NV12_BUF_CFG(pipe, plane_id),
> - &ddb->y_plane[pipe][plane_id]);
> + if (wm->is_nv12) {
> + skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
> + &ddb->uv_plane[pipe][plane_id]);
> + skl_ddb_entry_write(dev_priv,
> + PLANE_NV12_BUF_CFG(pipe, plane_id),
> + &ddb->plane[pipe][plane_id]);
> + } else {
> + skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
> + &ddb->plane[pipe][plane_id]);
> + skl_ddb_entry_write(dev_priv,
> + PLANE_NV12_BUF_CFG(pipe, plane_id),
> + &ddb->uv_plane[pipe][plane_id]);
It's probably better to always write 0 here to avoid confusion. :)
Writing something else here is probably a bad idea..
> + }
> }
>
> static void skl_write_cursor_wm(struct intel_crtc *intel_crtc,
> @@ -4906,8 +4906,8 @@ skl_ddb_add_affected_planes(struct intel_crtc_state *cstate)
>
> if (skl_ddb_entry_equal(&cur_ddb->plane[pipe][plane_id],
> &new_ddb->plane[pipe][plane_id]) &&
> - skl_ddb_entry_equal(&cur_ddb->y_plane[pipe][plane_id],
> - &new_ddb->y_plane[pipe][plane_id]))
> + skl_ddb_entry_equal(&cur_ddb->uv_plane[pipe][plane_id],
> + &new_ddb->uv_plane[pipe][plane_id]))
> continue;
>
> plane_state = drm_atomic_get_plane_state(state, plane);
> @@ -5001,8 +5001,8 @@ skl_copy_wm_for_pipe(struct skl_ddb_values *dst,
> struct skl_ddb_values *src,
> enum pipe pipe)
> {
> - memcpy(dst->ddb.y_plane[pipe], src->ddb.y_plane[pipe],
> - sizeof(dst->ddb.y_plane[pipe]));
> + memcpy(dst->ddb.uv_plane[pipe], src->ddb.uv_plane[pipe],
> + sizeof(dst->ddb.uv_plane[pipe]));
> memcpy(dst->ddb.plane[pipe], src->ddb.plane[pipe],
> sizeof(dst->ddb.plane[pipe]));
> }
With that fixed for this and patch 1:
Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
More information about the Intel-gfx
mailing list