[Intel-gfx] DP compliance failure due to dithering for 18bpp video pattern

[Jani Nikula]

On Fri, 13 Jan 2017, Manasi Navare <manasi.d.navare at intel.com> wrote:
> On Thu, Jan 12, 2017 at 03:41:07PM -0800, Rodrigo Vivi wrote:
>> On Thu, Jan 12, 2017 at 3:30 PM, Manasi Navare
>> <manasi.d.navare at intel.com> wrote:
>> > On Thu, Jan 12, 2017 at 12:32:09PM +0200, Jani Nikula wrote:
>> >> On Wed, 11 Jan 2017, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
>> >> > On Wed, Jan 11, 2017 at 05:09:16PM +0200, Jani Nikula wrote:
>> >> >> On Tue, 10 Jan 2017, Manasi Navare <manasi.d.navare at intel.com> wrote:
>> >> >> > Hi All,
>> >> >> >
>> >> >> > We are seeing CRC check failures in some of the 18bpp video pattern
>> >> >> > DP Compliance tests causing the tests to fail. On further investigation, it is
>> >> >> > rootcaused to dithering that the i915 driver enables in case of 18bpp pipe
>> >> >> > configuration that messes up the CRC and causes the test to fail.
>> >> >>
>> >> >> The CTS spec actually accounts for CRC failures caused by dithering and
>> >> >> color space conversions. See section 3.2.1. However, it would be
>> >> >> preferrable to be able to automate this.
>> >> >>
>> >> >> > Some of the approaches that can solve this problem are:
>> >> >> > 1.  Add a new method in intel_dp.c to request the compliance test state.
>> >> >> > Call this new method in intel_display.c to not enable dithering during a
>> >> >> > compliance test. Issue with this is it makes the general portion of the driver
>> >> >> > compliance aware.
>> >> >> >
>> >> >> > 2.  Move the dithering enable to compute_config methods in all encoder source
>> >> >> > files. Issue: Lot of duplicate code and DP is the only encoder that uses 18bpc.
>> >> >> >
>> >> >> > 3.  Disable dithering at all times in the driver. However this can cause image
>> >> >> > quality issue with 8bpc plane and 6 bit pipe.
>> >> >> >
>> >> >> > Any suggestions on which approach can be implemented in order to pass
>> >> >> > compliance?
>> >> >>
>> >> >> I can't find any mention in the specs that we couldn't enable/disable
>> >> >> dithering on the fly. It's PIPE_MISC for BDW+ and PIPE_CONF for the
>> >> >> rest. So I'm wondering about doing...
>> >> >>
>> >> >> 4. Disable dithering at intel_dp_sink_crc_start() and enable it again
>> >> >>    (according to config->dither) at intel_dp_sink_crc_stop(). It's
>> >> >>    similar to the hsw_disable_ips() and hsw_enable_ips() calls, but
>> >> >>    would have to cover more platforms.
>> >> >>
>> >
>> > intel_dp_sink_crc_start() gets called only through the debugfs interface.
>> 
>> Do you really use this sink crc for the compliance tests?
>> Or are you talking about some other CRC level check like in the compliance box?
>>
>
> Exactly, DPR 120 does a CRC check on the pixel values during the 18bpp video pattern
> tests. So we need to disable dithering before that modeset or during intel_modeset_pipe_config()
> where it sets pipe_config->dither to true.
> So I am not sure I understand how disabling the dithering at intel_dp_sink_crc_start() would help.

Oh. I thought the test involved reading the DPCD for the CRC. I didn't
realize the test sink not only calculates the CRC for the source to read
from DPCD, but also makes a point of checking it.

If so, we do need to disable dithering at modeset.

Again, this is not a test failure according to the CTS (there's a
mention of a visual inspection) but automation FTW.

BR,
Jani.


>
> Manasi 
>> > If we disable dithering here, DPR 120 which is calculating the CRC of the
>> > pixels on the rendered video pattern would already have dithering enabled on it.
>> > So how will this solve our issue of CRC failures in case of 18bpc test?
>> >
>> > Manasi
>> >
>> >
>> >> >> Ville, thoughts on changing dithering on the fly?
>> >> >
>> >> > Should be fine I think.
>> >> >
>> >> > BTW see
>> >> > https://lists.freedesktop.org/archives/intel-gfx/2016-December/115186.html
>> >> > if you intend to add more crc workaround type of things. There I'm
>> >> > changing the IPS w/a to force a full modeset because it was the easiest
>> >> > way to do things, and the current thing is just broken.
>> >>
>> >> I think forcing a modeset for sink crc would be rather annoying.
>> >>
>> >> BR,
>> >> Jani.
>> >>
>> >>
>> >> --
>> >> Jani Nikula, Intel Open Source Technology Center
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> 
>> 
>> -- 
>> Rodrigo Vivi
>> Blog: http://blog.vivi.eng.br

-- 
Jani Nikula, Intel Open Source Technology Center