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

Srinivas, Vidya
2018-01-30T04:05:14Z

> -----Original Message-----
> From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
> Sent: Monday, January 29, 2018 10:47 PM
> To: Srinivas, Vidya <vidya.srinivas@intel.com>; intel-
> gfx@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@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)
> Hey,
>
> Has NV12 been tested at all on skylake with rotation?
>
> y_min_scanlines = 16 for 90°/270° rotation with cpp=1
> skl_needs_memory_bw_wa() doubles the scanlines required, which will
> trigger the following error quite easily when patching kms_rotation_crc to
> use NV12:
>
> [   67.049190] [drm:skl_compute_wm_levels [i915]] Requested display
> configuration exceeds system watermark limitations [   67.049212]
> [drm:skl_compute_wm_levels [i915]] [PLANE:28:plane 1A] blocks required =
> 161/572, lines required = 32/31
>
> If we violate the spec for the workaround, by using 31 lines instead of 32 for
> 90/270 rotation, the tests work but this needs to be solved first before we
> can move forward.
>
We came across many such scenarios during NV12 testing on BXT. During downscaling,
the "Max supported pixel clock with scaling exceeded" is seen if the user layer tries to downscale
much. And rotation, as you mentioned we have seen such messages. We discussed the same during the meeting
long back and mentioned that if we remove the limitations, it works. But as per discussion, if limitations are removed, it may work,
but there may be certain conditions where it may not work. Hence we concluded that some such cases are expected not to work.
However, I will discuss once again with Mahesh Kumar (CC) who works on WM implementation to re-confirm on the same.

> ~Maarten