Op 22-01-18 om 13:03 schreef Vidya Srinivas:
> From: Mahesh Kumar <mahesh1.ku...@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.
>
> v2: Addressed review comments by Maarten
>
> Reviewed-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
> Signed-off-by: Mahesh Kumar <mahesh1.ku...@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srini...@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 4 +-
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 121
> ++++++++++++++++++++-------------------
> 3 files changed, 64 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d440a11..df91904 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1433,8 +1433,8 @@ 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 */
> - struct skl_ddb_entry y_plane[I915_MAX_PIPES][I915_MAX_PLANES];
> + struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES]; /*
> packed/y */
> + 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 5391c77..fc48697 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -592,6 +592,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 b4cb21d..bed5de0 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,17 @@ 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 +4089,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 +4100,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 +4120,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 +4153,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 +4168,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 +4186,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 +4211,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 +4221,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 +4243,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 +4274,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 +4415,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 +4607,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 +4780,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]);
I915_WRITE(PLANE_NV12_BUF_CFG(pipe, plane_id), 0); for !NV12?
much clearer..
Otherwise looks good so after we have tests it should be appliable with this
fixed up by the committer.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
