On Fri, Sep 11, 2015 at 09:44:18AM -0700, Chandra Konduru wrote:
> This patch adds NV12 as supported format to
> intel_framebuffer_init and performs various checks.
>
> v2:
> -Fix an issue in checks added (me)
>
> v3:
> -cosmetic update, split checks into two (Ville)
>
> v4:
> -Add stride alignment and modifier checks for UV subplane (Ville)
>
> v5:
> -Make modifier check general (Ville)
> -Check tile-y uv start alignment from begining of page (Ville)
>
> Signed-off-by: Chandra Konduru <chandra.konduru@xxxxxxxxx>
> Testcase: igt/kms_nv12
> ---
> drivers/gpu/drm/i915/intel_display.c | 66 +++++++++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_sprite.c | 2 +-
> 3 files changed, 55 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2a5170e..af28ca9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2914,9 +2914,9 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
> }
>
> u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
> - uint32_t pixel_format)
> + uint32_t pixel_format, int plane)
> {
> - u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, 0) * 8;
> + u32 bits_per_pixel = drm_format_plane_cpp(pixel_format, plane) * 8;
>
> /*
> * The stride is either expressed as a multiple of 64 bytes
> @@ -3125,7 +3125,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>
> obj = intel_fb_obj(fb);
> stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> - fb->pixel_format);
> + fb->pixel_format, 0);
> surf_addr = intel_plane_obj_offset(to_intel_plane(plane), obj, 0);
>
> /*
> @@ -9104,7 +9104,7 @@ skylake_get_initial_plane_config(struct intel_crtc *crtc,
>
> val = I915_READ(PLANE_STRIDE(pipe, 0));
> stride_mult = intel_fb_stride_alignment(dev, fb->modifier[0],
> - fb->pixel_format);
> + fb->pixel_format, 0);
> fb->pitches[0] = (val & 0x3ff) * stride_mult;
>
> aligned_height = intel_fb_align_height(dev, fb->height,
> @@ -11175,7 +11175,7 @@ static void skl_do_mmio_flip(struct intel_crtc *intel_crtc)
> */
> stride = fb->pitches[0] /
> intel_fb_stride_alignment(dev, fb->modifier[0],
> - fb->pixel_format);
> + fb->pixel_format, 0);
>
> /*
> * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
> @@ -14241,6 +14241,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
> {
> unsigned int aligned_height;
> int ret;
> + int i;
> u32 pitch_limit, stride_alignment;
>
> WARN_ON(!mutex_is_locked(&dev->struct_mutex));
> @@ -14255,7 +14256,8 @@ static int intel_framebuffer_init(struct drm_device *dev,
> }
> } else {
> if (obj->tiling_mode == I915_TILING_X)
> - mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED;
> + for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++)
> + mode_cmd->modifier[i] = I915_FORMAT_MOD_X_TILED;
The other branch needs updating too so that it will reject the operation
if the modifier disagrees with the obj tiling mode.
> else if (obj->tiling_mode == I915_TILING_Y) {
> DRM_DEBUG("No Y tiling for legacy addfb\n");
> return -EINVAL;
> @@ -14280,12 +14282,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
> return -EINVAL;
> }
>
> - stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[0],
> - mode_cmd->pixel_format);
> - if (mode_cmd->pitches[0] & (stride_alignment - 1)) {
> - DRM_DEBUG("pitch (%d) must be at least %u byte aligned\n",
> - mode_cmd->pitches[0], stride_alignment);
> - return -EINVAL;
> + /* check stride alignment for sub-planes */
> + for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) {
> + stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[i],
> + mode_cmd->pixel_format, i);
> + if (mode_cmd->pitches[i] & (stride_alignment - 1)) {
> + DRM_DEBUG("subplane %d pitch (%d) must be at least %u bytes "
> + "aligned\n", i, mode_cmd->pitches[i], stride_alignment);
> + return -EINVAL;
> + }
> }
>
> pitch_limit = intel_fb_pitch_limit(dev, mode_cmd->modifier[0],
> @@ -14352,9 +14357,44 @@ static int intel_framebuffer_init(struct drm_device *dev,
> return -EINVAL;
> }
> break;
> + case DRM_FORMAT_NV12:
> + if (INTEL_INFO(dev)->gen < 9) {
> + DRM_DEBUG("unsupported pixel format: %s\n",
> + drm_get_format_name(mode_cmd->pixel_format));
> + return -EINVAL;
> + }
> + if (!mode_cmd->offsets[1]) {
> + DRM_DEBUG("uv start offset not set\n");
> + return -EINVAL;
> + }
> + if (mode_cmd->pitches[0] != mode_cmd->pitches[1]) {
> + DRM_DEBUG("y and uv subplanes have different pitches\n");
> + return -EINVAL;
> + }
> + if (mode_cmd->handles[0] != mode_cmd->handles[1]) {
> + DRM_DEBUG("y and uv subplanes have different handles\n");
> + return -EINVAL;
> + }
> + if (mode_cmd->modifier[0] != mode_cmd->modifier[1]) {
> + DRM_DEBUG("y and uv subplanes have different modifiers\n");
> + return -EINVAL;
> + }
> + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED &&
> + (mode_cmd->offsets[1] & 0xFFF)) {
I've been trying to solicit ideas on how we should define the offsets[];
raw byte offset, or linear offset. I didn't get many opinions yet. So we
need to figure it out and document it somewhere before we expose it to
the world. In the meantime we could just reject non tile row aligned
offsets regardless of the tiling mode.
> + DRM_DEBUG("tile-Yf uv offset 0x%x isn't starting on new tile-row\n",
> + mode_cmd->offsets[1]);
> + return -EINVAL;
> + }
> + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Y_TILED &&
> + (((mode_cmd->offsets[1] & 0xFFF) / mode_cmd->pitches[1]) % 4)) {
> + DRM_DEBUG("tile-Y uv offset 0x%x isn't 4-line aligned\n",
> + mode_cmd->offsets[1]);
> + return -EINVAL;
> + }
> + break;
> default:
> DRM_DEBUG("unsupported pixel format: %s\n",
> - drm_get_format_name(mode_cmd->pixel_format));
> + drm_get_format_name(mode_cmd->pixel_format));
> return -EINVAL;
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d50b8cb..62d2a11 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -980,7 +980,7 @@ unsigned int intel_fb_align_height(struct drm_device *dev,
> void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire,
> enum fb_op_origin origin);
> u32 intel_fb_stride_alignment(struct drm_device *dev, uint64_t fb_modifier,
> - uint32_t pixel_format);
> + uint32_t pixel_format, int plane);
>
> /* intel_audio.c */
> void intel_init_audio(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 797594e..49feae0 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -203,7 +203,7 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
> src_w != crtc_w || src_h != crtc_h);
>
> stride_div = intel_fb_stride_alignment(dev, fb->modifier[0],
> - fb->pixel_format);
> + fb->pixel_format, 0);
>
> scaler_id = to_intel_plane_state(drm_plane->state)->scaler_id;
>
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
