Re: [Intel-gfx] [PATCH 10/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Ville Syrjälä]

On Wed, Aug 19, 2015 at 06:02:31PM -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)
> 
> Signed-off-by: Chandra Konduru 
> Testcase: igt/kms_nv12
> ---
>  drivers/gpu/drm/i915/intel_display.c |   28 
>  1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index e4a6a91..4df4d77 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14343,6 +14343,34 @@ 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;
> + }

I'm not sure this check makes much sense. We should perhaps either reject
all cases where the planes overlap, or allow it all.

Some years ago I was thinking of adding overlap check to drm core, but
then I figured maybe someone wants to interleave the planes in memory,
so the overlap checks would then have to check each line for overlap
so it gets a bit nasty. So I'm not sure there's any point in disallowing
overlapping planes.

> + if (mode_cmd->pitches[0] != mode_cmd->pitches[1] ||
> + mode_cmd->handles[0] != mode_cmd->handles[1]) {
> + DRM_DEBUG("y and uv subplanes have different 
> parameters\n");
> + return -EINVAL;
> + }

Maybe we'd want a separate debug message for these two. I think the
shared stride limitation would be fairly common in other hardware, but
the fact that we require the bo to be the same is unfortunately quite
a special "feature" of our latest hardware. So having a clear debug
message for it might help people figure out why their seemingly valid
code doesn't work on i915.

> + if (mode_cmd->modifier[1] == I915_FORMAT_MOD_Yf_TILED &&
> + (mode_cmd->offsets[1] & 0xFFF)) {

Bunch of indentation problems in this patch as well.

> + 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] / 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));
> -- 
> 1.7.9.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 10/15] drm/i915: Add NV12 support to intel_framebuffer_init

[Chandra Konduru]

This patch adds NV12 as supported format to
intel_framebuffer_init and performs various checks.

v2:
-Fix an issue in checks added (me)

Signed-off-by: Chandra Konduru chandra.kond...@intel.com
Testcase: igt/kms_nv12
---
 drivers/gpu/drm/i915/intel_display.c |   28 
 1 file changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index e4a6a91..4df4d77 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14343,6 +14343,34 @@ 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] ||
+   mode_cmd-handles[0] != mode_cmd-handles[1]) {
+   DRM_DEBUG(y and uv subplanes have different 
parameters\n);
+   return -EINVAL;
+   }
+   if (mode_cmd-modifier[1] == I915_FORMAT_MOD_Yf_TILED 
+   (mode_cmd-offsets[1]  0xFFF)) {
+   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] / 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));
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx