On Wed, Jun 17, 2015 at 07:52:14PM -0700, Doug Anderson wrote:
Russell,
On Wed, Jun 17, 2015 at 4:30 PM, Russell King - ARM Linux linux@arm.linux.org.uk wrote:
On Wed, Jun 17, 2015 at 04:14:07PM -0700, Doug Anderson wrote:
If you plug in a DVI monitor to your HDMI port, you need to filter out clocks > 165MHz. That's because 165MHz is the maximum clock rate that we can run single-link DVI at.
If you want to run high resolutions to DVI, you'd need some type of an active adapter that pretended that it was HDMI, interpreted the signal, and produced a new dual link DVI signal at a lower clock rate.
Signed-off-by: Doug Anderson dianders@chromium.org
Note: this patch was tested against a 3.14 kernel with backports. It was only compile tested against linuxnext, but the code is sufficiently similar that I'm convinced it will work there.
Really? I have to wonder what your testing was...
hdmi->vic = drm_match_cea_mode(mode); if (!hdmi->vic) { dev_dbg(hdmi->dev, "Non-CEA mode used in HDMI\n"); hdmi->hdmi_data.video_mode.mdvi = true; } else { dev_dbg(hdmi->dev, "CEA mode used vic=%d\n", hdmi->vic); hdmi->hdmi_data.video_mode.mdvi = false; }
mdvi indicates whether the _currently set mode_ is a CEA mode or not (imho, it's mis-named). It doesn't indicate whether we have a HDMI display device or a DVI display device connected, which seems to be what you want to use it for below.
To sort that, what you need to do is detect a HDMI display device using drm_detect_hdmi_monitor() on the EDID received from the device before parsing the modes, and save that value in a dw_hdmi struct member, and I'd suggest that it's a top-level struct member, not buried in 'hdmi_data' or 'video_mode'.
OK, so clearly my patch won't work against mainline. I guess it's a good thing that I pointed out that it was only tested locally (would have been better to test against mainline, but I don't think that's so easy since there are several unlanded patches in mainline for Rockchip).
As far as I'm aware, Freescale's original BSP version was the same, as is their later BSPs, and Jon's maintained 3.14-stable kernel.
As pointed out by others at http://crosreview.com/278255, locally our kernel has a slightly older version of https://lkml.org/lkml/2015/2/28/291, which would change mdvi to be as needed.
Please don't post unreliable lkml.org URLs, please use some other archive site. I can't access this URL at the moment.
...so I guess my change is blocked on someone reviewing/landing that series. If that series is rejected (or is changed sufficiently so that mdvi no longer is set via drm_detect_hdmi_monitor() then my patch will need to be re-spun.
That's not what I said. I said mdvi is set according to whether the mode being set is a CEA mode or not. We need something set according to the return value of drm_detect_hdmi_monitor(), which will tell us if the connected sink is a HDMI device or a DVI device (based upon the EDID.)
A thought occurs to me this morning though: what happens if you connect a DVI monitor to an AV receiver which is then connected to this device. Does the resulting EDID contain the HDMI vendor ID? If it does, it means that drm_detect_hdmi_monitor() will return true, indicating that the connected device is HDMI, and we will still allow modes greater than 165MHz.
That's probably a scenario that should be checked at some point... and it would throw a question mark over whether this is the correct approach to limit the video modes.