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

[Srinivas, Vidya]

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst at linux.intel.com]
> Sent: Monday, January 29, 2018 5:12 PM
> To: Srinivas, Vidya <vidya.srinivas at intel.com>; intel-
> gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 15/15] drm/i915: Add NV12 support to
> intel_framebuffer_init
> 
> Op 20-01-18 om 22:45 schreef Vidya Srinivas:
> > From: Chandra Konduru <chandra.konduru at 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 at intel.com>
> > Reviewed-by: Clinton Taylor <clinton.a.taylor at intel.com>
> > Signed-off-by: Chandra Konduru <chandra.konduru at intel.com>
> > Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti at intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas at 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?

I think, under display buffer programming it is mentioned that: For NV12 Bpp is 1 for Y and 2 for UV.
This is for all SKL+, it says.
> 
> ~Maarten