On Thu, Jun 18, 2015 at 08:26:39AM -0700, Doug Anderson wrote:
Russell,
On Thu, Jun 18, 2015 at 1:53 AM, Russell King - ARM Linux linux@arm.linux.org.uk wrote:
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.
Was "the same"? You mean was untested, or was 3.14?
I'm saying that the mdvi thing behaves the same in all the kernel sources I've seen of this driver, and I'm unaware of anything that changes it - and I've been looking at Jon's 3.14-stable kernels as well as Freescale's git repository.
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.
Perhaps you can try https://patchwork.kernel.org/patch/5906771/
Something like that needs to be done, but let's get rid of the mdvi thing in struct hdmi_vmode - it doesn't belong there, it isn't part of the currently set video mode, but becomes a property of the connected sink.
I'd also prefer it to be called "is_dvi_sink", especially as its function is changing from "is it a CEA mode" to "is the attached device a DVI sink".
Even better would be to call it "is_hdmi_sink" to maintain positive logic with single-negation where required, rather than double- negation in places.
...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.
Perhaps now that you can access the patch with the patchwork link you can re-read my email. If/when that patch lands then mdvi _will_ be set as per drm_detect_hdmi_monitor().
Well, I object to that patch (see above.)
I am nowhere near an HDMI expert. If you have a better suggestion then I'm more than happy for you to post it and drop my patch. In my non-expert opinion, it would seem awfully strange for an AV receiver to modify the EDID though unless it was actively interpreting the signal and generating a whole new signal on the other end. In any case, perhaps you can find such a device and that will give insight to how we should deal with it. Until such a device is found, it seems fruitless to speculate.
Neither am I, but I have had the ability to do some testing with AV receivers in the path of a HDMI device, and I've seen how they behave. (I made copious notes on this, which I intend to publish when I have a round tuit.) Unfortunately, I have no DVI devices to test with, and DVI devices are a dying breed - most monitors today come with HDMI sockets instead.
Personally, I was pointed at "drivers/gpu/drm/i915/intel_hdmi.c". If you look there you will find a similar bit of code.
Yea, I've also been using that for inspiration too, but I put personal testing above what's in someone elses driver. :)
To summarize: I am not planning to spin my patch. I am hopeful that folks could review Yakir's series. Would it help if he re-sent it with different people in the "To" line?
That's a shame... I'm not inclined to Ack it as-is - and I'd also like to see Yakir's patch reworked as I mentioned above.