Bug 93361 - [IVB] 12bpc hdmi causes wrong real refresh rate (swapbuffers return time)
Summary: [IVB] 12bpc hdmi causes wrong real refresh rate (swapbuffers return time)
Status: RESOLVED FIXED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: DRI git
Hardware: x86-64 (AMD64) Linux (All)
: medium critical
Assignee: Radhakrishna Sripada
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard: Triaged, ReadyForDev
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-12 02:18 UTC by Kevin Mitchell
Modified: 2019-02-28 11:29 UTC (History)
5 users (show)

See Also:
i915 platform: IVB
i915 features: display/HDMI


Attachments
Samsung un40eh5000 edid reported by xrandr --prop (513 bytes, text/plain)
2015-12-12 02:18 UTC, Kevin Mitchell
no flags Details
above edid decoded by edid-decode (3.19 KB, text/plain)
2015-12-12 02:18 UTC, Kevin Mitchell
no flags Details
dmesg log (109.27 KB, text/plain)
2017-02-09 16:34 UTC, Matt Horan
no flags Details
dmesg with HDMI-1 Samsung display (222.10 KB, text/plain)
2017-02-11 11:08 UTC, Kevin Mitchell
no flags Details
patch that adds debug logging (1.98 KB, patch)
2017-02-13 15:29 UTC, Tomeu Vizoso
no flags Details | Splinter Review
dmesg log with debug logging (53.83 KB, text/x-log)
2017-02-14 03:32 UTC, Matt Horan
no flags Details
dmesg with debuging patch on 4.10-rc6 with HDMI-1 Samsung display (207.94 KB, text/plain)
2017-02-14 05:28 UTC, Kevin Mitchell
no flags Details
patch that adds debug logging (2.90 KB, message/news)
2017-02-14 14:38 UTC, Tomeu Vizoso
no flags Details
dmesg log with more debug logging (60.23 KB, text/x-log)
2017-02-15 13:06 UTC, Matt Horan
no flags Details
dmesg log with more debug logging and drm.debug flag (116.09 KB, text/x-log)
2017-02-17 03:14 UTC, Matt Horan
no flags Details
dmesg with more debuging patch on drm-intel HEAD with HDMI-1 Samsung display (210.35 KB, text/plain)
2017-02-18 06:39 UTC, Kevin Mitchell
no flags Details
EDID for Sony TV via Rotel RSX-1058 receiver (256 bytes, application/octet-stream)
2017-02-18 14:44 UTC, Matt Horan
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin Mitchell 2015-12-12 02:18:29 UTC
Created attachment 120472 [details]
Samsung un40eh5000 edid reported by xrandr --prop

I have a Samsung un40eh5000 tv that I connect to my Ivy Bridge Lenovo T530 laptop via mini display-port to hdmi. I have attached the raw and decoded EDID. 

As of 7a0baa6234468aa387f9b8a1a79dc2a4b4821f67, it seems 12bpc gets enabled on this setup (though I can't imagine that my machine or my TV actually support that in real life). Unfortunately, this seems to change the effective refresh rate as measured between returns from opengl swapbuffers calls for example in glxgears.

       | glxgears fps   |
       +----------------+
xrandr | before | after |
-------+--------+-------+
23.98  | 24.00  | 23.85 |
24.00  | 24.00  | 24.13 |
29.97  | 30.00  | 29.81 |
30.00  | 30.00  | 30.17 |
59.94  | 60.00  | 60.12 |
60.00  | 60.00  | 60.12 |

This is problematic for displaying video at native frame rates. For example when the actual refresh rate of 24.00 hz could be achieved, 23.98fps video would only have to repeat a frame every 50 seconds. Now it must either drop or repeat one every 7 seconds which is much more noticeable.

This is duplicated from https://bugzilla.kernel.org/show_bug.cgi?id=109141 where I was told to post here instead.
Comment 1 Kevin Mitchell 2015-12-12 02:18:51 UTC
Created attachment 120473 [details]
above edid decoded by edid-decode
Comment 2 Matt Horan 2016-02-26 03:46:07 UTC
I'm experiencing a similar issue, except that in my case, my TV displays an error when 12bpc is enabled.

I have my computer connected to an HDMI receiver, which is in turn connected to my TV. While my TV supports 12bpc, my HDMI receiver does not. My guess is that the EDID is getting mangled somewhere, causing the Intel driver to pick up the wrong mode.

When I connect my computer directly to my TV, all is fine. 

I also traced this back to 7a0baa6234468aa387f9b8a1a79dc2a4b4821f67. All is fine before this commit, and goes haywire after.
Comment 3 Jari Tahvanainen 2017-01-23 08:48:51 UTC
Ricardo - evaluate if keywords needs to be updated.
Kevin, Matt - this has been flying under our radar, sorry.
Comment 4 Tomeu Vizoso 2017-02-09 12:37:39 UTC
Kevin, Matt, could you please test with drm.debug=0xe and attach the dmesg log?
Comment 5 Matt Horan 2017-02-09 16:34:36 UTC
Created attachment 129443 [details]
dmesg log

Attached dmesg log from my system.
Comment 6 Kevin Mitchell 2017-02-11 11:08:02 UTC
Created attachment 129503 [details]
dmesg with HDMI-1 Samsung display
Comment 7 Tomeu Vizoso 2017-02-13 15:29:08 UTC
Created attachment 129555 [details] [review]
patch that adds debug logging

Sorry, but I'm not able to get the whole picture from those logs.

Would it be possible for any of you to apply this patch to drm-tip and run it with drm_debug=0xe?
Comment 8 Matt Horan 2017-02-14 03:32:16 UTC
Created attachment 129573 [details]
dmesg log with debug logging

I couldn't get the provided patch to apply against drm-tip, but I did manage to patch the files manually.

Attached is my dmesg.log with debug logging.
Comment 9 Kevin Mitchell 2017-02-14 05:28:55 UTC
Created attachment 129578 [details]
dmesg with debuging patch on 4.10-rc6 with HDMI-1 Samsung display
Comment 10 Tomeu Vizoso 2017-02-14 14:38:48 UTC
Created attachment 129600 [details]
patch that adds debug logging

Big thanks to both.

The logs clearly point to a failure to clamp the pipe's max bpp value according to the sink's max bpc (in connected_sink_compute_bpp()), but I don't really see yet how that would be possible.

Could you please do the same with this new patch? It should apply cleanly to drm-tip: 2017y-02m-14d-12h-28m-14s.

And please remember to boot with drm.debug=0xe.
Comment 11 Matt Horan 2017-02-15 13:06:29 UTC
Created attachment 129625 [details]
dmesg log with more debug logging

Updated dmesg log.
Comment 12 Tomeu Vizoso 2017-02-15 15:47:34 UTC
(In reply to Matt Horan from comment #11)
> Created attachment 129625 [details]
> dmesg log with more debug logging
> 
> Updated dmesg log.

Could you please reboot with drm.debug=0xe instead of drm_debug=0xe? Note the dot instead of the underscore. Thanks!
Comment 13 Matt Horan 2017-02-17 03:14:44 UTC
Created attachment 129686 [details]
dmesg log with more debug logging and drm.debug flag
Comment 14 Tomeu Vizoso 2017-02-17 14:16:32 UTC
(In reply to Kevin Mitchell from comment #0)
> Created attachment 120472 [details]
> Samsung un40eh5000 edid reported by xrandr --prop
> 
> I have a Samsung un40eh5000 tv that I connect to my Ivy Bridge Lenovo T530
> laptop via mini display-port to hdmi. I have attached the raw and decoded
> EDID. 
> 
> As of 7a0baa6234468aa387f9b8a1a79dc2a4b4821f67, it seems 12bpc gets enabled
> on this setup (though I can't imagine that my machine or my TV actually
> support that in real life).

Why do you think that? The EDID says that DC_36bit is supported, and from sandy bridge up, 12bpc is supported by the graphics hw.

> Unfortunately, this seems to change the
> effective refresh rate as measured between returns from opengl swapbuffers
> calls for example in glxgears.
> 
>        | glxgears fps   |
>        +----------------+
> xrandr | before | after |
> -------+--------+-------+
> 23.98  | 24.00  | 23.85 |
> 24.00  | 24.00  | 24.13 |
> 29.97  | 30.00  | 29.81 |
> 30.00  | 30.00  | 30.17 |
> 59.94  | 60.00  | 60.12 |
> 60.00  | 60.00  | 60.12 |
> 
> This is problematic for displaying video at native frame rates. For example
> when the actual refresh rate of 24.00 hz could be achieved, 23.98fps video
> would only have to repeat a frame every 50 seconds. Now it must either drop
> or repeat one every 7 seconds which is much more noticeable.

So you want a mechanism for disabling deep color even if it could be used?
Comment 15 Tomeu Vizoso 2017-02-17 14:19:41 UTC
(In reply to Matt Horan from comment #13)
> Created attachment 129686 [details]
> dmesg log with more debug logging and drm.debug flag

Thanks, could you please attach the edid in binary format as it is in /sys/devices/pci0000:00/0000:00:02.0/drm/card0/card0-HDMI-A-2/edid (changing to the correct pci ID for your machine)?
Comment 16 Kevin Mitchell 2017-02-18 06:39:48 UTC
Created attachment 129723 [details]
dmesg with more debuging patch on drm-intel HEAD with HDMI-1 Samsung display
Comment 17 Kevin Mitchell 2017-02-18 06:59:51 UTC
(In reply to Tomeu Vizoso from comment #14)
> (In reply to Kevin Mitchell from comment #0)
> > Created attachment 120472 [details]
> > Samsung un40eh5000 edid reported by xrandr --prop
> > 
> > I have a Samsung un40eh5000 tv that I connect to my Ivy Bridge Lenovo T530
> > laptop via mini display-port to hdmi. I have attached the raw and decoded
> > EDID. 
> > 
> > As of 7a0baa6234468aa387f9b8a1a79dc2a4b4821f67, it seems 12bpc gets enabled
> > on this setup (though I can't imagine that my machine or my TV actually
> > support that in real life).
> 
> Why do you think that? The EDID says that DC_36bit is supported, and from
> sandy bridge up, 12bpc is supported by the graphics hw.

Well I assume that if my TV could do deep colour, it would have been advertised as such and cost a lot more money. But to confirm, I see bands when I use mpv on the drm-intel git master kernel to play 10-bit test clips like

http://www.avsforum.com/forum/139-display-calibration/2269338-10-bit-gradient-test-patterns.html

Maybe the TVs DSP can take deep color as input, but it only displays as 8-bit on the panel. Do I need special xorg settings / patches to get this to work correctly?

> > Unfortunately, this seems to change the
> > effective refresh rate as measured between returns from opengl swapbuffers
> > calls for example in glxgears.
> > 
> >        | glxgears fps   |
> >        +----------------+
> > xrandr | before | after |
> > -------+--------+-------+
> > 23.98  | 24.00  | 23.85 |
> > 24.00  | 24.00  | 24.13 |
> > 29.97  | 30.00  | 29.81 |
> > 30.00  | 30.00  | 30.17 |
> > 59.94  | 60.00  | 60.12 |
> > 60.00  | 60.00  | 60.12 |
> > 
> > This is problematic for displaying video at native frame rates. For example
> > when the actual refresh rate of 24.00 hz could be achieved, 23.98fps video
> > would only have to repeat a frame every 50 seconds. Now it must either drop
> > or repeat one every 7 seconds which is much more noticeable.
> 
> So you want a mechanism for disabling deep color even if it could be used?

It would be great if this was just enabled where it was supported and there were no problems, but if it has the possibility to cause regressions like this, that might be an idea. That is effectively what I am doing now by reverting this change in my kernel.
Comment 18 Matt Horan 2017-02-18 14:44:21 UTC
Created attachment 129730 [details]
EDID for Sony TV via Rotel RSX-1058 receiver

Here is the EDID for my Sony TV as presented by my Rotel receiver. Note that, IIRC, the EDID shows 12bpp support, and the TV does in fact support 12bpp.

However, my Rotel RSX-1058 receiver only supports HDMI 1.1, thus preventing 12bpp support.

I've worked around this issue by editing the EDID to disable 12bpp support. This is done by the drm_kms_helper.edid_firmware kernel flag.
Comment 19 Tomeu Vizoso 2017-02-20 14:15:42 UTC
(In reply to Matt Horan from comment #18)
> Created attachment 129730 [details]
> EDID for Sony TV via Rotel RSX-1058 receiver
> 
> Here is the EDID for my Sony TV as presented by my Rotel receiver. Note
> that, IIRC, the EDID shows 12bpp support, and the TV does in fact support
> 12bpp.
> 
> However, my Rotel RSX-1058 receiver only supports HDMI 1.1, thus preventing
> 12bpp support.
> 
> I've worked around this issue by editing the EDID to disable 12bpp support.
> This is done by the drm_kms_helper.edid_firmware kernel flag.

Thanks, I have created bug 99869 for tracking your issue.
Comment 20 Tomeu Vizoso 2017-02-21 08:55:58 UTC
Kevin, I think this bug should remain open until the i915 driver becomes able to get a more accurate clock with 12bpc. This should be possible by using clock bending to match the desired refresh rate but this is a change that would considerably complicate clock handling in the code and so far nobody has started work on this.

That said, I'm reducing the priority to medium because there's a workaround that restores the previous behavior (replacing the edid with drm_kms_helper.edid_firmware), and my understanding is that the Highest priority is reserved for regressions that don't have a known workaround.
Comment 21 Jari Tahvanainen 2017-05-16 09:09:15 UTC
Removed regression related keywords. Sorry going back and forth, since I was the one adding those.
Comment 22 Elizabeth 2017-07-28 16:32:00 UTC
(In reply to Tomeu Vizoso from comment #20)
> Kevin, I think this bug should remain open until the i915 driver becomes
> able to get a more accurate clock with 12bpc. This should be possible by
> using clock bending to match the desired refresh rate but this is a change
> that would considerably complicate clock handling in the code and so far
> nobody has started work on this.
> 
> That said, I'm reducing the priority to medium because there's a workaround
> that restores the previous behavior (replacing the edid with
> drm_kms_helper.edid_firmware), and my understanding is that the Highest
> priority is reserved for regressions that don't have a known workaround.
Good afternoon,
Is there any update in this case? Is the accuracy improved in latest updates?
Thank you.
Comment 23 Jani Saarinen 2018-03-12 15:46:21 UTC
Reporters, any updates from you?
Comment 24 Ville Syrjala 2018-03-12 15:47:55 UTC
Nothing has changed in the relevant code, so no change in outcome is expected.
Comment 25 Jani Saarinen 2018-03-29 07:11:28 UTC
First of all. Sorry about spam.
This is mass update for our bugs. 

Sorry if you feel this annoying but with this trying to understand if bug still valid or not.
If bug investigation still in progress, please ignore this and I apologize!

If you think this is not anymore valid, please comment to the bug that can be closed.
If you haven't tested with our latest pre-upstream tree(drm-tip), can you do that also to see if issue is valid there still and if you cannot see issue there, please comment to the bug.
Comment 26 Jani Saarinen 2018-04-25 06:32:52 UTC
Keep this open still.
Comment 27 Stanislav Lisovskiy 2018-07-09 10:11:21 UTC
What about this bug? Should we now proceed with DRM property for explicit 12bpc disabling for such case, so that there is at least no need in hacking the EDID data? 
Or as I understand there is an option to implement a clock bending, which is order of magnitude higher in complexity.
Comment 28 Kevin Mitchell 2018-07-10 02:36:02 UTC
(In reply to Stanislav Lisovskiy from comment #27)
> What about this bug? Should we now proceed with DRM property for explicit
> 12bpc disabling for such case, so that there is at least no need in hacking
> the EDID data? 
> Or as I understand there is an option to implement a clock bending, which is
> order of magnitude higher in complexity.

It sounds like clock bending is the proper fix. As you can see from the table in the description, the refresh rate can be wrong even without the 12bbc complications. 12bpc just makes it more wrong. That said, it would be nice to have some user control over 12bpc behaviour in the interim.
Comment 29 Jani Saarinen 2018-09-24 06:19:31 UTC
Moving to RK.
Comment 30 Ville Syrjala 2019-01-22 20:43:07 UTC
The max bpc property was merged a while back:

xrandr --output <whatever> --set 'max bpc' 8

should do the trick to force 8bpc
Comment 31 James Ausmus 2019-02-27 22:39:01 UTC
With the 'max bpc' property being merged, is there anything additional here, or should this be closed?
Comment 32 Ville Syrjala 2019-02-28 11:29:50 UTC
(In reply to James Ausmus from comment #31)
> With the 'max bpc' property being merged, is there anything additional here,
> or should this be closed?

Yeah, user can choose between two wrong refresh rates now :P

Unlikely anyone will actually implement clock bending, unless someone gets bored and does it out of curiosity.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.