On Fri, Jan 13, 2017 at 09:11:49AM +0200, Jani Nikula wrote: > On Fri, 13 Jan 2017, Manasi Navare <manasi.d.navare@xxxxxxxxx> 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@xxxxxxxxx> wrote: > >> > On Thu, Jan 12, 2017 at 12:32:09PM +0200, Jani Nikula wrote: > >> >> On Wed, 11 Jan 2017, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > >> >> > On Wed, Jan 11, 2017 at 05:09:16PM +0200, Jani Nikula wrote: > >> >> >> On Tue, 10 Jan 2017, Manasi Navare <manasi.d.navare@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx Jani/Ville, So if I were to use the same method as HSW IPS, then I would add a field force_dither_disable to pipe_config and then set this to true in compute_config if it is compliance test request for 18bpp. Does this sound like a good approach? What I am confused about is do I need to force a full modeset for this force_dither_disable to actually disable pipe's dither or just setting that at compute_config() is enough? Manasi > >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >> > >> > >> > >> -- > >> Rodrigo Vivi > >> Blog: http://blog.vivi.eng.br > > -- > Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
