Re: [PATCH] drm/i915: Adding dbuf support for skl nv12 format.

Damien Lespiau
2015-05-08T17:23:47Z

On Mon, Apr 27, 2015 at 03:47:37PM -0700, Chandra Konduru wrote:
> Skylake nv12 format requires dbuf (aka. ddb) calculations
> and programming for each of y and uv sub-planes. Made minor
> changes to reuse current dbuf calculations and programming
> for uv plane. i.e., with this change, existing computation
> is used for either packed format or uv portion of nv12
> depending on incoming format. Added new code for dbuf
> computation and programming for y plane.
>
> This patch is a pre-requisite for adding NV12 format support.
> Actual nv12 support is coming in later patches.
>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>

Talked with Chandra about the previous comments. The Y tiling problem I
was mentioning isn't quite part of NV12 and can be fixed on top of this.
I'll probaby try my suggestions and create a series on top of this
patch.

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

--
Damien

> ---
> drivers/gpu/drm/i915/i915_drv.h | 3 +-
> drivers/gpu/drm/i915/i915_reg.h | 11 ++++++
> drivers/gpu/drm/i915/intel_drv.h | 8 ++++
> drivers/gpu/drm/i915/intel_pm.c | 79 ++++++++++++++++++++++++++++++++------
> 4 files changed, 88 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b13c552..75eb3a5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1450,7 +1450,8 @@ static inline bool skl_ddb_entry_equal(const struct skl_ddb_entry *e1,
>
> struct skl_ddb_allocation {
> struct skl_ddb_entry pipe[I915_MAX_PIPES];
> - struct skl_ddb_entry plane[I915_MAX_PIPES][I915_MAX_PLANES];
> + 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]; /* y-plane */
> struct skl_ddb_entry cursor[I915_MAX_PIPES];
> };
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0865ec7..aa2a0de 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4880,6 +4880,8 @@ enum skl_disp_power_wells {
> #define _PLANE_KEYMAX_2_A 0x702a0
> #define _PLANE_BUF_CFG_1_A 0x7027c
> #define _PLANE_BUF_CFG_2_A 0x7037c
> +#define _PLANE_NV12_BUF_CFG_1_A 0x70278
> +#define _PLANE_NV12_BUF_CFG_2_A 0x70378
>
> #define _PLANE_CTL_1_B 0x71180
> #define _PLANE_CTL_2_B 0x71280
> @@ -4966,6 +4968,15 @@ enum skl_disp_power_wells {
> #define PLANE_BUF_CFG(pipe, plane) \
> _PLANE(plane, _PLANE_BUF_CFG_1(pipe), _PLANE_BUF_CFG_2(pipe))
>
> +#define _PLANE_NV12_BUF_CFG_1_B 0x71278
> +#define _PLANE_NV12_BUF_CFG_2_B 0x71378
> +#define _PLANE_NV12_BUF_CFG_1(pipe) \
> + _PIPE(pipe, _PLANE_NV12_BUF_CFG_1_A, _PLANE_NV12_BUF_CFG_1_B)
> +#define _PLANE_NV12_BUF_CFG_2(pipe) \
> + _PIPE(pipe, _PLANE_NV12_BUF_CFG_2_A, _PLANE_NV12_BUF_CFG_2_B)
> +#define PLANE_NV12_BUF_CFG(pipe, plane) \
> + _PLANE(plane, _PLANE_NV12_BUF_CFG_1(pipe), _PLANE_NV12_BUF_CFG_2(pipe))
> +
> /* SKL new cursor registers */
> #define _CUR_BUF_CFG_A 0x7017c
> #define _CUR_BUF_CFG_B 0x7117c
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c9bc975..8079865 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -566,7 +566,15 @@ struct intel_crtc {
> struct intel_plane_wm_parameters {
> uint32_t horiz_pixels;
> uint32_t vert_pixels;
> + /*
> + * For packed pixel formats:
> + * bytes_per_pixel - holds bytes per pixel
> + * For planar pixel formats:
> + * bytes_per_pixel - holds bytes per pixel for uv-plane
> + * y_bytes_per_pixel - holds bytes per pixel for y-plane
> + */
> uint8_t bytes_per_pixel;
> + uint8_t y_bytes_per_pixel;
> bool enabled;
> bool scaled;
> u64 tiling;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 0f4391e..b8df120 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2616,8 +2616,18 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
> }
>
> static unsigned int
> -skl_plane_relative_data_rate(const struct intel_plane_wm_parameters *p)
> +skl_plane_relative_data_rate(const struct intel_plane_wm_parameters *p, int y)
> {
> +
> + /* for planar format */
> + if (p->y_bytes_per_pixel) {
> + if (y) /* y-plane data rate */
> + return p->horiz_pixels * p->vert_pixels * p->y_bytes_per_pixel;
> + else /* uv-plane data rate */
> + return (p->horiz_pixels/2) * (p->vert_pixels/2) * p->bytes_per_pixel;
> + }
> +
> + /* for packed formats */
> return p->horiz_pixels * p->vert_pixels * p->bytes_per_pixel;
> }
>
> @@ -2640,7 +2650,10 @@ skl_get_total_relative_data_rate(struct intel_crtc *intel_crtc,
> if (!p->enabled)
> continue;
>
> - total_data_rate += skl_plane_relative_data_rate(p);
> + total_data_rate += skl_plane_relative_data_rate(p, 0); /* packed/uv */
> + if (p->y_bytes_per_pixel) {
> + total_data_rate += skl_plane_relative_data_rate(p, 1); /* y-plane */
> + }
> }
>
> return total_data_rate;
> @@ -2659,6 +2672,7 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
> struct skl_ddb_entry *alloc = &ddb->pipe[pipe];
> uint16_t alloc_size, start, cursor_blocks;
> uint16_t minimum[I915_MAX_PLANES];
> + uint16_t y_minimum[I915_MAX_PLANES];
> unsigned int total_data_rate;
> int plane;
>
> @@ -2687,6 +2701,8 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
>
> minimum[plane] = 8;
> alloc_size -= minimum[plane];
> + y_minimum[plane] = p->y_bytes_per_pixel ? 8 : 0;
> + alloc_size -= y_minimum[plane];
> }
>
> /*
> @@ -2700,16 +2716,17 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
> start = alloc->start;
> for (plane = 0; plane < intel_num_planes(intel_crtc); plane++) {
> const struct intel_plane_wm_parameters *p;
> - unsigned int data_rate;
> - uint16_t plane_blocks;
> + unsigned int data_rate, y_data_rate;
> + uint16_t plane_blocks, y_plane_blocks = 0;
>
> p = &params->plane[plane];
> if (!p->enabled)
> continue;
>
> - data_rate = skl_plane_relative_data_rate(p);
> + data_rate = skl_plane_relative_data_rate(p, 0);
>
> /*
> + * allocation for (packed formats) or (uv-plane part of planar format):
> * promote the expression to 64 bits to avoid overflowing, the
> * result is < available as data_rate / total_data_rate < 1
> */
> @@ -2721,6 +2738,22 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
> ddb->plane[pipe][plane].end = start + plane_blocks;
>
> start += plane_blocks;
> +
> + /*
> + * allocation for y_plane part of planar format:
> + */
> + if (p->y_bytes_per_pixel) {
> + y_data_rate = skl_plane_relative_data_rate(p, 1);
> + y_plane_blocks = y_minimum[plane];
> + y_plane_blocks += div_u64((uint64_t)alloc_size * y_data_rate,
> + total_data_rate);
> +
> + ddb->y_plane[pipe][plane].start = start;
> + ddb->y_plane[pipe][plane].end = start + y_plane_blocks;
> +
> + start += y_plane_blocks;
> + }
> +
> }
>
> }
> @@ -2833,13 +2866,18 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
> p->pixel_rate = skl_pipe_pixel_rate(intel_crtc->config);
>
> fb = crtc->primary->state->fb;
> + /* For planar: Bpp is for uv plane, y_Bpp is for y plane */
> if (fb) {
> p->plane[0].enabled = true;
> - p->plane[0].bytes_per_pixel = fb->bits_per_pixel / 8;
> + p->plane[0].bytes_per_pixel = fb->pixel_format == DRM_FORMAT_NV12 ?
> + drm_format_plane_cpp(fb->pixel_format, 1) : fb->bits_per_pixel / 8;
> + p->plane[0].y_bytes_per_pixel = fb->pixel_format == DRM_FORMAT_NV12 ?
> + drm_format_plane_cpp(fb->pixel_format, 0) : 0;
> p->plane[0].tiling = fb->modifier[0];
> } else {
> p->plane[0].enabled = false;
> p->plane[0].bytes_per_pixel = 0;
> + p->plane[0].y_bytes_per_pixel = 0;
> p->plane[0].tiling = DRM_FORMAT_MOD_NONE;
> }
> p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w;
> @@ -2847,6 +2885,7 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
> p->plane[0].rotation = crtc->primary->state->rotation;
>
> fb = crtc->cursor->state->fb;
> + p->cursor.y_bytes_per_pixel = 0;
> if (fb) {
> p->cursor.enabled = true;
> p->cursor.bytes_per_pixel = fb->bits_per_pixel / 8;
> @@ -2882,22 +2921,25 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> uint32_t plane_bytes_per_line, plane_blocks_per_line;
> uint32_t res_blocks, res_lines;
> uint32_t selected_result;
> + uint8_t bytes_per_pixel;
>
> if (latency == 0 || !p->active || !p_params->enabled)
> return false;
>
> + bytes_per_pixel = p_params->y_bytes_per_pixel ?
> + p_params->y_bytes_per_pixel :
> + p_params->bytes_per_pixel;
> method1 = skl_wm_method1(p->pixel_rate,
> - p_params->bytes_per_pixel,
> + bytes_per_pixel,
> latency);
> method2 = skl_wm_method2(p->pixel_rate,
> p->pipe_htotal,
> p_params->horiz_pixels,
> - p_params->bytes_per_pixel,
> + bytes_per_pixel,
> p_params->tiling,
> latency);
>
> - plane_bytes_per_line = p_params->horiz_pixels *
> - p_params->bytes_per_pixel;
> + plane_bytes_per_line = p_params->horiz_pixels * bytes_per_pixel;
> plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
>
> if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
> @@ -3114,10 +3156,14 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv,
> new->plane_trans[pipe][i]);
> I915_WRITE(CUR_WM_TRANS(pipe), new->cursor_trans[pipe]);
>
> - for (i = 0; i < intel_num_planes(crtc); i++)
> + for (i = 0; i < intel_num_planes(crtc); i++) {
> skl_ddb_entry_write(dev_priv,
> PLANE_BUF_CFG(pipe, i),
> &new->ddb.plane[pipe][i]);
> + skl_ddb_entry_write(dev_priv,
> + PLANE_NV12_BUF_CFG(pipe, i),
> + &new->ddb.y_plane[pipe][i]);
> + }
>
> skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe),
> &new->ddb.cursor[pipe]);
> @@ -3275,6 +3321,7 @@ static bool skl_update_pipe_wm(struct drm_crtc *crtc,
> return false;
>
> intel_crtc->wm.skl_active = *pipe_wm;
> +
> return true;
> }
>
> @@ -3368,8 +3415,16 @@ skl_update_sprite_wm(struct drm_plane *plane, struct drm_crtc *crtc,
> intel_plane->wm.scaled = scaled;
> intel_plane->wm.horiz_pixels = sprite_width;
> intel_plane->wm.vert_pixels = sprite_height;
> - intel_plane->wm.bytes_per_pixel = pixel_size;
> intel_plane->wm.tiling = DRM_FORMAT_MOD_NONE;
> +
> + /* For planar: Bpp is for UV plane, y_Bpp is for Y plane */
> + intel_plane->wm.bytes_per_pixel =
> + (fb && fb->pixel_format == DRM_FORMAT_NV12) ?
> + drm_format_plane_cpp(plane->state->fb->pixel_format, 1) : pixel_size;
> + intel_plane->wm.y_bytes_per_pixel =
> + (fb && fb->pixel_format == DRM_FORMAT_NV12) ?
> + drm_format_plane_cpp(plane->state->fb->pixel_format, 0) : 0;
> +
> /*
> * Framebuffer can be NULL on plane disable, but it does not
> * matter for watermarks if we assume no tiling in that case.
> --
> 1.7.9.5