Re: [PATCH 15/15] drm/i915: Add NV12 support to intel_framebuffer_init

Maarten Lankhorst
2018-01-29T11:41:36Z

Op 20-01-18 om 22:45 schreef Vidya Srinivas:
> From: Chandra Konduru <chandra.konduru@intel.com>
>
> This patch adds NV12 as supported format
> to intel_framebuffer_init and performs various checks.
>
> v2:
> -Fix an issue in checks added (Chandra Konduru)
>
> v3: rebased (me)
>
> v4: Review comments by Ville addressed
> Added platform check for NV12 in intel_framebuffer_init
> Removed offset checks for NV12 case
>
> v5: Addressed review comments by Clinton A Taylor
> This NV12 support only correctly works on SKL.
> Plane color space conversion is different on GLK and later platforms
> causing the colors to display incorrectly.
> Ville's plane color space property patch series
> in review will fix this issue.
> - Restricted the NV12 case in intel_framebuffer_init to
> SKL and BXT only.
>
> v6: Rebased (me)
>
> v7: Addressed review comments by Ville
> Restricting the NV12 to BXT for now.
>
> v8: Rebased (me)
> Restricting the NV12 changes to BXT and KBL for now.
>
> v9: Rebased (me)
>
> Tested-by: Clinton Taylor <clinton.a.taylor@intel.com>
> Reviewed-by: Clinton Taylor <clinton.a.taylor@intel.com>
> Signed-off-by: Chandra Konduru <chandra.konduru@intel.com>
> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 88bc750..db42448 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14005,6 +14005,14 @@ static int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> goto err;
> }
> break;
> + case DRM_FORMAT_NV12:
> + if (!IS_BROXTON(dev_priv) && !IS_KABYLAKE(dev_priv)) {
> + DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> + drm_get_format_name(mode_cmd->pixel_format,
> + &format_name));
> + goto err;
> + }
> + break;
> default:
> DRM_DEBUG_KMS("unsupported pixel format: %s\n",
> drm_get_format_name(mode_cmd->pixel_format, &format_name));

Hey,

When implementing this for IGT I've noticed a small gap in documentation, which will definitely need clarification somewhere. intel_framebuffer_init is probably good enough since it's specific to how intel works internally..
The Yf tiling mode depends on BPP, is it correct to say that plane 0 has 8 bpp and plane 1 16 bpp for tiling calculations?

What bpp is used?

Are macroblock sizes always the same for Y and UV?

~Maarten