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

[Kumar, Mahesh]

Hi,

On 1/30/2018 9:35 AM, Srinivas, Vidya wrote:
>
>> -----Original Message-----
>> From: Maarten Lankhorst [mailto:maarten.lankhorst at linux.intel.com]
>> Sent: Monday, January 29, 2018 10:47 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)
>> 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 are hitting this issue due to memory_bw_wa (this is applicable for 
GEN-9 only) & we are applying WA irrespective of it's required or not.
I had plan to apply this WA based on our discussion, but it never 
materialized :(
https://www.spinics.net/lists/intel-gfx/msg124872.html

> 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.
Either we can conclude that as a limitation (as expected behavior) OR we 
can enable WA only when it's required (as discussed, assuming all the 
planes are enabled with max supported scaling to avoid taking mutex in 
each flip). what you say?

-Mahesh
> However, I will discuss once again with Mahesh Kumar (CC) who works on WM implementation to re-confirm on the same.
>
>> ~Maarten