On 8/13/2014 11:46 AM, Chris Wilson wrote:
On Wed, Aug 13, 2014 at 11:34:02AM +0530, Sharma, Shashank wrote:
I know. That is orthogonal to the tweaks I was suggesting. Also if you
feel you need to add details to your rationale, then your changelog is
incomplete.
-Chris
Thanks Chris,
Please find my comments inline to your previous mail, with suggestions.
On 8/12/2014 6:17 PM, Chris Wilson wrote:
On Tue, Aug 12, 2014 at 06:08:21PM +0530, shashank.sharma@xxxxxxxxx wrote:
From: Shashank Sharma <shashank.sharma@xxxxxxxxx>
During the HDMI complaince tests, most of the HDMI analyzers
issue a soft HPD, to automate the tests. This process keeps
the HDMI cable connected, and DDC chhanel alive.
HDMI detect() logic relies on EDID readability, to decide if
its a HDMI connect or disconnect event. But in this case, the
EDID is always readable, as HDMI cable is physically connected
and DDC channel is UP, so detect() always reports a HDMI connect
even if its intended to be a HDMI disconnect call.
So if HDMI compliance is enabled, we should rely on the live status
register, than EDID availability. This patch adds:
1. One kernel command line parameter for i915 module, which indicates
if we want to support HDMI compliance, for this platform.
I would rather have this as an output property. In fact, I would like
the hotplug detection method exposed (and even selectable, but other
than this compliance testing, I can't think of a scenario where the
kernel shouldn't be able to figure things out for itself).
Considering the history of the case, can you please elaborate this
suggestion ? I dont think I am getting it right.
Instead of (or in addition to) adding a kernel parameter, you add an
output property so that it can be adjusted on the fly for individual
monitors.
Yes, this can be done. This might cause some problems with the first
modeset from driver (like fb_console), where there is no userspace to
set a property yet, but I think its manageable.
2. A new function to read EDID, which gets called only in case of
HDMI compliance support is required. This function reads EDID only
if live status register is up. The normal code flow doesn't get effected
if kernel command line parameter is not enabled.
3. After various experiments on VLV2 board, with various HDMI monitors
we have seen that, with few monitors, the live status register gets
set after a slight delay, and then stays reliably. To support such
monitors, there is a busy-loop added, with a max delay upto 50ms,
with a status check after every 10ms. Please see the comment in
intel_hdmi_get_edid.
Wouldn't a tidier solution be to delay the hpd by 50-100ms after the
hotplug interrupt? That may overcome the issue with the live status for
all connectors...
-Chris
There would be few problems in this case:
1. We dont want this scenario to come into picture for DP, as DP HPD
pulse can be as small as 2ms.
If the live status is asserted and deasserted within 2ms, do you care?
Or perhaps you are talking about something else entirely.
Actually what I mean was, in case of compliance, there would be an
expectation for Port Up/Down/UP, but what we will get is UP/UP
This can fail the test case.
2. Not all the HDMI monitors show this problem, but a significant
subset of popular monitors do.
3. In HDCP compliance testing, we send a HPD pulse train of UP and
Down, where down pulse can be as small as 100ms. If we increase the
delay by 100ms, we will definitely miss the HPD down pulse.
And? If the monitor is only plugged in for less than 0.1s do I really
want to waste 1-2s of user time reconfiguring the desktop and
applications before undoing all the changes?
There is no point in compliance testing if it does not actually test the
code going to be used.
I completely agree, but unfortunately if the test case fails, this would
be blocker for commercial projects. The test clearly mentions we should
respond properly to the HPD pulse train.
4. The method what we are using is a busy waiting check, where we delay
the pulse for 50ms, but take a sample of live_status per 10ms, so if
the live status is up with a delay of 20ms, we needn't to waste
another 30.
Yes. You block using 100% of the cpu in an uninterruptable context for a
significant period of time. DO NOT DO THIS.
I can very well switch to a sleep, but was not sure if the context
switching 5 times for 10ms is beneficial :), please correct me if this
is not ok.
5. We want this code routine only to be executed for commercial (like
android) platforms, whereas others get the routine code.
In other words, you want to ignore years of real world compatibity testing
and larger user bases.
-Chris
I do not mean this for sure, in fact we would be happy if the community
accepts this for regular code flow also, but due to their previous
objections and stand for not to go for a live_status based solution,
made to to come via this route. Please suggest how can we do this in
better way.
Regards
Shashank
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
