For test 4.2.2.5 to pass per the Link CTS Core 1.2 rev1.1 spec, the source
device must attempt at least 7 times to read the EDID when it receives an
I2C defer. The normal DRM code makes only 7 retries, regardless of whether
or not the response is a native defer or an I2C defer. Test 4.2.2.5 fails
since there are native defers interspersed with the I2C defers which
results in less than 7 EDID read attempts.
The solution is to add the numer of defers to the retry counter when an I2C
DEFER is returned such that another read attempt will be made. This situation
should normally only occur in compliance testing, however, as a worse case
real-world scenario, it would result in 13 attempts ( 6 native defers, 7 I2C
defers) for a single transaction to complete. The net result is a slightly
slower response to an EDID read that shouldn't significantly impact overall
performance.
V2:
- Added a check on the number of I2C Defers to limit the number
of times that the retries variable will be decremented. This
is to address review feedback regarding possible infinite loops
from misbehaving sink devices.
V3:
- Fixed the limit value to 7 instead of 8 to get the correct retry
count.
- Combined the increment of the defer count into the if-statement
V4:
- Removed i915 tag from subject as the patch is not i915-specific
V5:
- Updated the for-loop to add the number of i2c defers to the retry
counter such that the correct number of retry attempts will be
made
Signed-off-by: Todd Previte <tprevite@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
The debug message is missing a newline at the end and it makes the
logs hard to read when a device defers a lot. Simple 2-character fix
adds the newline at the end.
Signed-off-by: Todd Previte <tprevite@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
When doing a native or i2c aux write the sink will indicate the number
of bytes written even if it the nacks the transfer. When we receive a
nack we just return an error upwards, but it might still be interesting
to see how many bytes made it before the nack. So include that information
in the debug messages.
v2: Also print the message size (Jani)
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Older DisplayPort to DVI-D Dual Link adapters designed by Bizlink have bugs
in their I2C over AUX implementation (fixed in newer revisions). They work
fine with Windows, but fail with Linux.
It turns out that they cannot keep an I2C transaction open unless the
previous read was 16 bytes; shorter reads can only be followed by a zero
byte transfer ending the I2C transaction.
Copy Windows's behaviour, and read 16 bytes at a time. If we get a short
reply, assume that there's a hardware bottleneck, and shrink our read size
to match. For this purpose, use the algorithm in the DisplayPort 1.2 spec,
in the hopes that it'll be closest to what Windows does.
Also provide an unsafe module parameter for testing smaller transfer sizes,
in case there are sinks out there that cannot work with Windows.
Note also that despite the previous comment in drm_dp_i2c_xfer, this speeds
up native DP EDID reads; Ville Syrjälä <ville.syrjala@linux.intel.com> found
the following changes in his testing:
Device under test: old -> with this patch
DP->DVI (OUI 001cf8): 40ms -> 35ms
DP->VGA (OUI 0022b9): 45ms -> 38ms
Zotac DP->2xHDMI: 25ms -> 4ms
Asus PB278 monitor: 22ms -> 3ms
A back of the envelope calculation shows that peak theoretical transfer rate
for 1 byte reads is around 60 kbit/s; with 16 byte reads, this increases to
around 500 kbit/s, which explains the increase in speed.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=55228
Tested-by: Aidan Marks <aidanamarks@gmail.com> (v3)
Signed-off-by: Simon Farnsworth <simon.farnsworth@onelan.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
We had _power_up(), but drivers also need to be able to power down.
Signed-off-by: Rob Clark <robdclark@gmail.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
At least on two MST devices I've tested with, when
they are link training downstream, they are totally
unable to handle aux ch msgs, so they defer like nuts.
I tried 16, it wasn't enough, 32 seems better.
This fixes one Dell 4k monitor and one of the
MST hubs.
v1.1: fixup comment (Tom).
Acked-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
Signed-off-by: Dave Airlie <airlied@redhat.com>
These counters are used for Displayort compliance testing to detect error
conditions when executing tests 4.2.2.4 and 4.2.2.5 in the Displayport Link
CTS specificaiton. They determine whether to use the preferred/requested
mode or the failsafe mode during these tests.
V2:
- Addressed previous review feedback
- Updated commit message
- Changed from uint8_t to uint32_t
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Todd Previte <tprevite@gmail.com>
[danvet: s/uint32_t/unsigned/ for clearer intent. Also drop the i915
from the subject, it's all core stuff.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Except for gma500 all drivers are converted to the new style helpers,
which have much better abstraction of the underlying hw protocols and
already much more helper functions (including the entire mst library)
on top of them. Since no one seems to work on converting gma500 let's
just move the code away so that new drivers don't end up accidentally
using this.
Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Reviewed-by: Alan Cox <alan@linux.intel.com>
[danvet: Add __deprecated as requested by Alan. Also add a short FIXME
comment and drop the EXPORT_SYMBOL which is no longer needed.]
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
This should avoid races between connector probing and HPD
irqs in the future, currently mode_config.mutex blocks this
possibility.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
Only gma500 is still using this, once that's converted we can kill all
this code. If that conversion doesn't happen soonish I think we should
just move this helper code into the gma500 driver itself to avoid
abuse from new drivers.
Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Replace an occurrence of "adapater" with "adapter".
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
If you are using the common dp over i2c functionality, it is
asumed that the aux transfer function does not modify the any
of the msg structure other than the reply field. Doing so
breaks the logic in the common code.
v2: update struct drm_dp_aux comments about assumptions
v3 (chk): rebased on upstream changes
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Thierry Reding <treding@nvidia.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Christian König <christian.koenig@amd.com>
We need bare address packets at the start and end of
each i2c over aux transaction to properly reset the connection
between transactions. This mirrors what the existing dp i2c
over aux algo currently does.
This fixes EDID fetches on certain monitors especially with
dp bridges.
v2: update as per Ville's comments
- Set buffer to NULL for zero sized packets
- abort the entre transaction if one of the messages fails
v3: drop leftover debugging code
v4: integrate Thierry's comments
- add comments about address only transactions
- switch back to i and j
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Thierry Reding <treding@nvidia.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Christian König <christian.koenig@amd.com>
If we get a msg.reply of REPLY_DEFER, we also get an err of 0
so we fail reads with 0 < size and return -EPROTO instead of trying
again.
v2: same fix in i2c code.
Found writing MST support.
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
- Inherit/reuse firmwar framebuffers (for real this time) from Jesse, less
flicker for fastbooting.
- More flexible cloning for hdmi (Ville).
- Some PPGTT fixes from Ben.
- Ring init fixes from Naresh Kumar.
- set_cache_level regression fixes for the vma conversion from Ville&Chris.
- Conversion to the new dp aux helpers (Jani).
- Unification of runtime pm with pc8 support from Paulo, prep work for runtime
pm on other platforms than HSW.
- Larger cursor sizes (Sagar Kamble).
- Piles of improvements and fixes all over, as usual.
* tag 'drm-intel-next-2014-03-21' of git://anongit.freedesktop.org/drm-intel: (75 commits)
drm/i915: Include a note about the dangers of I915_READ64/I915_WRITE64
drm/i915/sdvo: fix questionable return value check
drm/i915: Fix unsafe loop iteration over vma whilst unbinding them
drm/i915: Enabling 128x128 and 256x256 ARGB Cursor Support
drm/i915: Print how many objects are shared in per-process stats
drm/i915: Per-process stats work better when evaluated per-process
drm/i915: remove rps local variables
drm/i915: Remove extraneous MMIO for RPS
drm/i915: Rename and comment all the RPS *stuff*
drm/i915: Store the HW min frequency as min_freq
drm/i915: Fix coding style for RPS
drm/i915: Reorganize the overclock code
drm/i915: init pm.suspended earlier
drm/i915: update the PC8 and runtime PM documentation
drm/i915: rename __hsw_do_{en, dis}able_pc8
drm/i915: kill struct i915_package_c8
drm/i915: move pc8.irqs_disabled to pm.irqs_disabled
drm/i915: remove dev_priv->pc8.enabled
drm/i915: don't get/put PC8 when getting/putting power wells
drm/i915: make intel_aux_display_runtime_get get runtime PM, not PC8
...
Conflicts:
drivers/gpu/drm/i915/intel_display.c
drivers/gpu/drm/i915/intel_dp.c
Switch to debug only to avoid flooding the logs.
This mirrors the behavior in some other drivers.
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Christian König <christian.koenig@amd.com>
Let the drivers specify the name of the I2C-over-AUX adapter to maintain
backwards compatibility in the sysfs when converting to the new
I2C-over-AUX helper infrastructure.
The i915 driver currently uses DPDDC-A to DPDDC-D as names for the DP
i2c adapters. These names show up in the i2c sysfs name attribute. We'd
like to be able to maintain that when switching over to the new helpers.
Due to i2c device and connector cleanup ordering issues we also recently
made the drm device (instead of connector) the parent of the i2c
adapters:
commit 80f65de3c9
Author: Imre Deak <imre.deak@intel.com>
Date: Tue Feb 11 17:12:49 2014 +0200
drm/i915: dp: fix order of dp aux i2c device cleanup
With the name picked up from the adapter parent using dev_name(), it
would be the same for all i2c adapters with the current I2C-over-AUX
helpers.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Thierry Reding <treding@nvidia.com>
Acked-by: Dave Airlie <airlied@redhat.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Implements an I2C-over-AUX I2C adapter on top of the generic drm_dp_aux
infrastructure. It extracts the retry logic from existing drivers, which
should help in porting those drivers to this new helper.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v5:
- move comments partially to to header file
- keep MOT set between I2C messages
- return -EPROTO on short reads
Changes in v4:
- fix typo "bitrate" -> "bit rate"
Changes in v3:
- add back DRM_DEBUG_KMS and DRM_ERROR messages
- embed i2c_adapter within struct drm_dp_aux
- fix typo in comment
Add a helper to probe a DP link (read out the supported DPCD revision,
maximum rate, link count and capabilities) as well as power up the DP
link and configure it accordingly.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v5:
- export helpers
Changes in v4:
- fix a couple of typos in comments as pointed out by Alex Deucher
Changes in v3:
- split into drm_dp_link_power_up() and drm_dp_link_configure()
- do not change sink state for DPCD versions earlier than 1.1
- sleep for 1-2 ms after setting local sink to D0 state
- read and write consecutive registers where possible
- read DPCD revision when link is probed
- remove duplicate kerneldoc
The function reads the link status (6 bytes starting at offset 0x202)
from the DPCD so that it can be conveniently passed to other DPCD
helpers.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
This is a superset of the current i2c_dp_aux bus functionality and can
be used to transfer native AUX in addition to I2C-over-AUX messages.
Helpers are provided to read and write the DPCD, either blockwise or
byte-wise. Many of the existing helpers for DisplayPort take a copy of a
portion of the DPCD and operate on that, without a way to write data
back to the DPCD (e.g. for configuration of the link).
Subsequent patches will build upon this infrastructure to provide common
functionality in a generic way.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v5:
- move comments partially to struct drm_dp_aux_msg in header file
- return -EPROTO on short reads in DPCD helpers
Changes in v4:
- fix a typo in a comment
Changes in v3:
- reorder drm_dp_dpcd_writeb() arguments to be more intuitive
- return number of bytes transferred in drm_dp_dpcd_write()
- factor out drm_dp_dpcd_access()
- describe error codes
None of the DP DPCD helpers need to modify the DPCD.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
I didn't bother with documenting the really trivial new "extract
something from dpcd" helpers, but the i2c over aux ch is now
documented a bit.
v2: Clarify the comment for i2c_dp_aux_add_bus a bit.
v3: Fix more spelling fail spotted by Laurent Pinchart.
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>
This requires a few changes since that dpcd value is above the
range currently cached by radeon. I've check the dp specs, and
above 0xf there's a big gap and nothing that looks like we should
cache it while a given device is plugged in. It's also the same value
that i915.ko uses.
Hence extend the various dpcd arrays in the radeon driver, use
proper symbolic constants where applicable (one place overallocated
the dpcd array to 25 bytes). Then also drop the rd_interval cache -
radeon_dp_link_train_init re-reads the dpcd block, so the values we'll
consume in train_cr and train_ce will always be fresh.
To avoid needless diff-churn, #define the old size of dpcd as the new
one and keep it around.
v2: Alex Deucher noticed one place where I've forgotten to replace 8
with DP_RECEIVER_CAP_SIZE.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Safe for the minor difference that the intel versions get an offset
into the link_status as an argument, both are the same again.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
radeon and intel use the exact same definition.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I want to move some dp link training helpers into this place, so in
the future this won't be just about i2c any longer.
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Todd Previte