From the CI builds, its been observed that during a driver
reload/insert, dp dual mode read function sometimes fails to
read from LSPCON device over i2c-over-aux channel.
This patch:
- adds some delay and few retries, allowing a scope for these
devices to settle down and respond.
- changes one error log's level from ERROR->DEBUG as we want
to call it an error only after all the retries are exhausted.
V2: Addressed review comments from Jani (for loop for retry)
V3: Addressed review comments from Imre (break on partial read too)
V3: Addressed review comments from Ville/Imre (Add the retries
exclusively for LSPCON, not for all dp_dual_mode devices)
V4: Added r-b from Imre, sending it to dri-devel (Jani)
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102294
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102295
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102359
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103186
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/1507826408-19322-1-git-send-email-shashank.sharma@intel.com
GFP_TEMPORARY was introduced by commit e12ba74d8f ("Group short-lived
and reclaimable kernel allocations") along with __GFP_RECLAIMABLE. It's
primary motivation was to allow users to tell that an allocation is
short lived and so the allocator can try to place such allocations close
together and prevent long term fragmentation. As much as this sounds
like a reasonable semantic it becomes much less clear when to use the
highlevel GFP_TEMPORARY allocation flag. How long is temporary? Can the
context holding that memory sleep? Can it take locks? It seems there is
no good answer for those questions.
The current implementation of GFP_TEMPORARY is basically GFP_KERNEL |
__GFP_RECLAIMABLE which in itself is tricky because basically none of
the existing caller provide a way to reclaim the allocated memory. So
this is rather misleading and hard to evaluate for any benefits.
I have checked some random users and none of them has added the flag
with a specific justification. I suspect most of them just copied from
other existing users and others just thought it might be a good idea to
use without any measuring. This suggests that GFP_TEMPORARY just
motivates for cargo cult usage without any reasoning.
I believe that our gfp flags are quite complex already and especially
those with highlevel semantic should be clearly defined to prevent from
confusion and abuse. Therefore I propose dropping GFP_TEMPORARY and
replace all existing users to simply use GFP_KERNEL. Please note that
SLAB users with shrinkers will still get __GFP_RECLAIMABLE heuristic and
so they will be placed properly for memory fragmentation prevention.
I can see reasons we might want some gfp flag to reflect shorterm
allocations but I propose starting from a clear semantic definition and
only then add users with proper justification.
This was been brought up before LSF this year by Matthew [1] and it
turned out that GFP_TEMPORARY really doesn't have a clear semantic. It
seems to be a heuristic without any measured advantage for most (if not
all) its current users. The follow up discussion has revealed that
opinions on what might be temporary allocation differ a lot between
developers. So rather than trying to tweak existing users into a
semantic which they haven't expected I propose to simply remove the flag
and start from scratch if we really need a semantic for short term
allocations.
[1] http://lkml.kernel.org/r/20170118054945.GD18349@bombadil.infradead.org
[akpm@linux-foundation.org: fix typo]
[akpm@linux-foundation.org: coding-style fixes]
[sfr@canb.auug.org.au: drm/i915: fix up]
Link: http://lkml.kernel.org/r/20170816144703.378d4f4d@canb.auug.org.au
Link: http://lkml.kernel.org/r/20170728091904.14627-1-mhocko@kernel.org
Signed-off-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Acked-by: Mel Gorman <mgorman@suse.de>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Neil Brown <neilb@suse.de>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Handle DRM_DP_DUAL_MODE_LSPCON in drm_dp_get_dual_mode_type_name(),
otherwise a call to that function can theoretically trigger a WARN.
Fixes: 056996b956 ("drm: Helper for lspcon in drm_dp_dual_mode")
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dave Airlie <airlied@gmail.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: http://patchwork.freedesktop.org/patch/msgid/20170222125928.30223-1-ander.conselvan.de.oliveira@intel.com
There's at least one LSPCON device that occasionally returns an unexpected
adaptor ID which leads to a failed detect. Print some debug info to help
debugging this and future cases. Also print an error for an unexpected
adaptor ID, so users can report it.
v2:
- s/adapter/adaptor/ and add code comment about incorrect type 1 adaptor
IDs. (Ville)
Cc: dri-devel@lists.freedesktop.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
Link: http://patchwork.freedesktop.org/patch/msgid/1477499359-12001-1-git-send-email-imre.deak@intel.com
Fixes sparse warning:
drivers/gpu/drm/drm_dp_dual_mode_helper.c:151:6: warning: symbol
'is_lspcon_adaptor' was not declared. Should it be static?
Fixes: 056996b956 ("drm: Helper for lspcon in drm_dp_dual_mode")
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: <dri-devel@lists.freedesktop.org>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1476789711-19697-1-git-send-email-jani.nikula@intel.com
This patch adds lspcon support in dp_dual_mode helper.
lspcon is essentially a dp->hdmi dongle with dual personality.
LS mode: It works as a passive dongle, by level shifting DP++
signals to HDMI signals, in LS mode.
PCON mode: It works as a protocol converter active dongle
in pcon mode, by converting DP++ outputs to HDMI 2.0 outputs.
This patch adds support for lspcon detection and mode set
switch operations, as a dp dual mode dongle.
v2: Addressed review comments from Ville
- add adaptor id for lspcon devices (0x08), use it to identify lspcon
- change function names
old: drm_lspcon_get_current_mode/drm_lspcon_change_mode
new: drm_lspcon_get_mode/drm_lspcon_set_mode
- change drm_lspcon_get_mode type to int, to match
drm_dp_dual_mode_get_tmds_output
- change 'err' to 'ret' to match the rest of the functions
- remove pointless typecasting during call to dual_mode_read
- fix the but while setting value of data, while writing lspcon mode
- fix indentation
- change mdelay(10) -> msleep(10)
- return ETIMEDOUT instead of EFAULT, when lspcon mode change times out
- Add an empty line to separate std regs macros and lspcon regs macros
Indent bit definition
v3: Addressed review comments from Rodrigo
- change macro name from DP_DUAL_MODE_TYPE_LSPCON to
DP_DUAL_MODE_TYPE_HAS_DPCD for better readability
- change macro name from DP_DUAL_MODE_LSPCON_MODE_PCON to
DP_DUAL_MODE_LSPCON_MODE_PCON for better readability
- add comment for MCA specific offsets like 0x40 and 0x41
- remove DP_DUAL_MODE_REV_TYPE2 check while checking lspcon adapter id
v4: Addressed review comments from Ville
- Fixed indentation at few places
- s/current_mode/mode
- s/reqd_mode/mode
- remove unnecessary void* cast
- remove drm_edid.h from includes
- Add a comment for _HAS_DPCD
- Fix enum description, for lspcon_mode.
v5: Rebase
v6: Rebase
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1476720277-16298-1-git-send-email-shashank.sharma@intel.com
Add a helper which aids in the identification of DP dual mode
(aka. DP++) adaptors. There are several types of adaptors
specified: type 1 DVI, type 1 HDMI, type 2 DVI, type 2 HDMI
Type 1 adaptors have a max TMDS clock limit of 165MHz, type 2 adaptors
may go as high as 300MHz and they provide a register informing the
source device what the actual limit is. Supposedly also type 1 adaptors
may optionally implement this register. This TMDS clock limit is the
main reason why we need to identify these adaptors.
Type 1 adaptors provide access to their internal registers and the sink
DDC bus through I2C. Type 2 adaptors provide this access both via I2C
and I2C-over-AUX. A type 2 source device may choose to implement either
of these methods. If a source device implements the I2C-over-AUX
method, then the driver will obviously need specific support for such
adaptors since the port is driven like an HDMI port, but DDC
communication happes over the AUX channel.
This helper should be enough to identify the adaptor type (some
type 1 DVI adaptors may be a slight exception) and the maximum TMDS
clock limit. Another feature that may be available is control over
the TMDS output buffers on the adaptor, possibly allowing for some
power saving when the TMDS link is down.
Other user controllable features that may be available in the adaptors
are downstream i2c bus speed control when using i2c-over-aux, and
some control over the CEC pin. I chose not to provide any helper
functions for those since I have no use for them in i915 at this time.
The rest of the registers in the adaptor are mostly just information,
eg. IEEE OUI, hardware and firmware revision, etc.
v2: Pass adaptor type to helper functions to ease driver implementation
Fix a bunch of typoes (Paulo)
Add DRM_DP_DUAL_MODE_UNKNOWN for the case where we don't (yet) know
the type (Paulo)
Reject 0x00 and 0xff DP_DUAL_MODE_MAX_TMDS_CLOCK values (Paulo)
Adjust drm_dp_dual_mode_detect() type2 vs. type1 detection to
ease future LSPCON enabling
Remove the unused DP_DUAL_MODE_LAST_RESERVED define
v3: Fix kernel doc function argument descriptions (Jani)
s/NONE/UNKNOWN/ in drm_dp_dual_mode_detect() docs
Add kernel doc for enum drm_dp_dual_mode_type
Actually build the docs
Fix more typoes
v4: Adjust code indentation of type2 adaptor detection (Shashank)
Add debug messages for failurs cases (Shashank)
v5: EXPORT_SYMBOL(drm_dp_dual_mode_read) (Paulo)
Cc: stable@vger.kernel.org
Cc: Tore Anderson <tore@fud.no>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> (v4)
Link: http://patchwork.freedesktop.org/patch/msgid/1462542412-25533-1-git-send-email-ville.syrjala@linux.intel.com
Shashank Sharma