Per toshiba,tc358767.yaml DT binding document, port@2 the output (e)DP
port is optional. In case this port is not described in DT, the bridge
driver operates in DPI-to-DP mode. The drm_of_find_panel_or_bridge()
call in tc_probe_edp_bridge_endpoint() returns -ENODEV in case port@2
is not present in DT and this specific return value is incorrectly
propagated outside of tc_probe_edp_bridge_endpoint() function. All
other error values must be propagated and are propagated correctly.
Return 0 in case the port@2 is missing instead, that reinstates the
original behavior before the commit this patch fixes.
Fixes: 8478095a8c ("drm/bridge: tc358767: Move (e)DP bridge endpoint parsing into dedicated function")
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Marek Vasut <marex@denx.de>
Cc: Maxime Ripard <maxime@cerno.tech>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Robert Foss <robert.foss@linaro.org>
Cc: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
Link: https://patchwork.freedesktop.org/patch/msgid/20220428213132.447890-1-marex@denx.de
Rename dp/ to display/ to account for additional display-related
helpers, such as HDMI. Update all related include statements. No
functional changes.
Various drivers, such as i915 and amdgpu, use similar naming scheme
by putting code for video-output standards into a local display/
directory. The new directory's name is aligned with this convention.
v2:
* update commit message (Javier)
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220421073108.19226-3-tzimmermann@suse.de
The TC358767/TC358867/TC9595 are all capable of operating in multiple
modes, DPI-to-(e)DP, DSI-to-(e)DP, DSI-to-DPI. Add support for the
DSI-to-DPI mode.
This requires skipping most of the (e)DP initialization code, which is
currently a large part of this driver, hence it is better to have far
simpler separate tc_dpi_bridge_funcs and their implementation.
The configuration of DPI output is also much simpler. The configuration
of the DSI input is rather similar to the other TC bridge chips.
The Pixel PLL in DPI output mode does not have the 65..150 MHz limitation
imposed on the (e)DP output mode, so this limitation is skipped to permit
operating panels with far slower pixel clock, even below 9 MHz. This mode
of operation of the PLL is valid and tested.
The detection of bridge mode is now added into tc_probe_bridge_mode(),
where in case a DPI panel is found on port@1 endpoint@1, the mode is
assumed to be DSI-to-DPI. If (e)DP is detected on port@2, the mode is
assumed to be DPI-to-(e)DP.
The DSI-to-(e)DP mode is not supported due to lack of proper hardware,
but this would be some sort of mix between the two aforementioned modes.
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Maxime Ripard <maxime@cerno.tech>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Robert Foss <robert.foss@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20220329085015.39159-12-marex@denx.de
The TC358767/TC358867/TC9595 are all capable of operating in multiple
modes, DPI-to-(e)DP, DSI-to-(e)DP, DSI-to-DPI. Only the first mode is
currently supported. It is possible to find out the mode in which the
bridge should be operated by testing connected endpoints in DT.
Port allocation:
port@0 - DSI input
port@1 - DPI input/output
port@2 - eDP output
Possible connections:
DPI -> port@1 -> port@2 -> eDP :: [port@0 is not connected]
DSI -> port@0 -> port@2 -> eDP :: [port@1 is not connected]
DSI -> port@0 -> port@1 -> DPI :: [port@2 is not connected]
Add function to determine the bridge mode based on connected endpoints.
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
Tested-by: Lucas Stach <l.stach@pengutronix.de> # In both DPI to eDP and DSI to DPI mode.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Maxime Ripard <maxime@cerno.tech>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Signed-off-by: Robert Foss <robert.foss@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20220329085015.39159-10-marex@denx.de
struct drm_display_mode embeds a list head, so overwriting
the full struct with another one will corrupt the list
(if the destination mode is on a list). Use drm_mode_copy()
instead which explicitly preserves the list head of
the destination mode.
Even if we know the destination mode is not on any list
using drm_mode_copy() seems decent as it sets a good
example. Bad examples of not using it might eventually
get copied into code where preserving the list head
actually matters.
Obviously one case not covered here is when the mode
itself is embedded in a larger structure and the whole
structure is copied. But if we are careful when copying
into modes embedded in structures I think we can be a
little more reassured that bogus list heads haven't been
propagated in.
@is_mode_copy@
@@
drm_mode_copy(...)
{
...
}
@depends on !is_mode_copy@
struct drm_display_mode *mode;
expression E, S;
@@
(
- *mode = E
+ drm_mode_copy(mode, &E)
|
- memcpy(mode, E, S)
+ drm_mode_copy(mode, E)
)
@depends on !is_mode_copy@
struct drm_display_mode mode;
expression E;
@@
(
- mode = E
+ drm_mode_copy(&mode, &E)
|
- memcpy(&mode, E, S)
+ drm_mode_copy(&mode, E)
)
@@
struct drm_display_mode *mode;
@@
- &*mode
+ mode
Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Robert Foss <robert.foss@linaro.org>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220218100403.7028-7-ville.syrjala@linux.intel.com
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
This is something that we've wanted for a while now: the ability to
actually look up the respective drm_device for a given drm_dp_aux struct.
This will also allow us to transition over to using the drm_dbg_*() helpers
for debug message printing, as we'll finally have a drm_device to reference
for doing so.
Note that there is one limitation with this - because some DP AUX adapters
exist as platform devices which are initialized independently of their
respective DRM devices, one cannot rely on drm_dp_aux->drm_dev to always be
non-NULL until drm_dp_aux_register() has been called. We make sure to point
this out in the documentation for struct drm_dp_aux.
v3:
* Add WARN_ON_ONCE() to drm_dp_aux_register() if drm_dev isn't filled out
Signed-off-by: Lyude Paul <lyude@redhat.com>
Acked-by: Thierry Reding <treding@nvidia.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210423184309.207645-4-lyude@redhat.com
Reviewed-by: Dave Airlie <airlied@redhat.com>
Since this is a bridge, we don't start out with a respective DRM device.
Likewise this means we don't have a connector, which also means that we
should be following drm_dp_aux_register()'s documentation advice and not
call drm_dp_aux_register() until we have a matching connector. Instead,
call drm_dp_aux_init() in tc_probe() and wait until tc_bridge_attach() to
register our AUX channel. We also add tc_bridge_detach() to handle
unregistering the AUX adapter once the bridge has been disconnected.
Signed-off-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Robert Foss <robert.foss@linaro.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20210219215326.2227596-5-lyude@redhat.com
With the bridge operations implemented the last step to prepare
this driver for a chained setup is the use of the bridge panel driver.
The bridge panel driver is only used when a port@2 is present in
the DT. So when the display driver requests a connector
support both situations:
- connector created by bridge panel driver
- connector created by this driver
And on top, support that the display driver creates the connector,
which is the preferred setup.
Note: the bridge panel will use the connector type from the panel.
v3:
- Fix wrong logic in connector creation (Laurent)
v2:
- Merge connector and drm_panel_bridge patches
and fix so we do not create two connectors (Laurent)
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Neil Armstrong <narmstrong@baylibre.com>
Cc: Jonas Karlman <jonas@kwiboo.se>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>
Link: https://patchwork.freedesktop.org/patch/msgid/20200727170320.959777-4-sam@ravnborg.org
When validating a mode, bridges may need to do so in the context of a
display, as specified by drm_display_info. An example is the meson
dw-hdmi bridge that needs to consider the YUV 4:2:0 output format to
perform clock calculations.
Bridges that need the display info currently retrieve it from the
drm_connector created by the bridge. This gets in the way of moving
connector creation out of bridge drivers. To make this possible, pass
the drm_display_info to drm_bridge_funcs .mode_valid().
Changes to the bridge drivers have been performed with the following
coccinelle semantic patch and have been compile-tested.
@ rule1 @
identifier funcs;
identifier fn;
@@
struct drm_bridge_funcs funcs = {
...,
.mode_valid = fn
};
@ depends on rule1 @
identifier rule1.fn;
identifier bridge;
identifier mode;
@@
enum drm_mode_status fn(
struct drm_bridge *bridge,
+ const struct drm_display_info *info,
const struct drm_display_mode *mode
)
{
...
}
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Guido Günther <agx@sigxcpu.org> # for the nwl-dsi part:
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20200526011505.31884-11-laurent.pinchart+renesas@ideasonboard.com
Most bridge drivers create a DRM connector to model the connector at the
output of the bridge. This model is historical and has worked pretty
well so far, but causes several issues:
- It prevents supporting more complex display pipelines where DRM
connector operations are split over multiple components. For instance a
pipeline with a bridge connected to the DDC signals to read EDID data,
and another one connected to the HPD signal to detect connection and
disconnection, will not be possible to support through this model.
- It requires every bridge driver to implement similar connector
handling code, resulting in code duplication.
- It assumes that a bridge will either be wired to a connector or to
another bridge, but doesn't support bridges that can be used in both
positions very well (although there is some ad-hoc support for this in
the analogix_dp bridge driver).
In order to solve these issues, ownership of the connector should be
moved to the display controller driver (where it can be implemented
using helpers provided by the core).
Extend the bridge API to allow disabling connector creation in bridge
drivers as a first step towards the new model. The new flags argument to
the bridge .attach() operation allows instructing the bridge driver to
skip creating a connector. Unconditionally set the new flags argument to
0 for now to keep the existing behaviour, and modify all existing bridge
drivers to return an error when connector creation is not requested as
they don't support this feature yet.
The change is based on the following semantic patch, with manual review
and edits.
@ rule1 @
identifier funcs;
identifier fn;
@@
struct drm_bridge_funcs funcs = {
...,
.attach = fn
};
@ depends on rule1 @
identifier rule1.fn;
identifier bridge;
statement S, S1;
@@
int fn(
struct drm_bridge *bridge
+ , enum drm_bridge_attach_flags flags
)
{
... when != S
+ if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
+ DRM_ERROR("Fix bridge driver to make connector optional!");
+ return -EINVAL;
+ }
+
S1
...
}
@ depends on rule1 @
identifier rule1.fn;
identifier bridge, flags;
expression E1, E2, E3;
@@
int fn(
struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags
) {
<...
drm_bridge_attach(E1, E2, E3
+ , flags
)
...>
}
@@
expression E1, E2, E3;
@@
drm_bridge_attach(E1, E2, E3
+ , 0
)
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Tested-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200226112514.12455-10-laurent.pinchart@ideasonboard.com
Link training fails with:
Link training timeout waiting for LT_LOOPDONE!
main link enable error: -110
This is caused by too tight timeouts, which were changed recently in
aa92213f38 ("drm/bridge: tc358767: Simplify polling in tc_link_training()").
With a quick glance, the commit does not change the timeouts. However,
the method of delaying/sleeping is different, and as the timeout in the
previous implementation was not explicit, the new version in practice
has much tighter timeout.
The same change was made to other parts in the driver, but the link
training timeout is the only one I have seen causing issues.
Nevertheless, 1 us sleep is not very sane, and the timeouts look pretty
tight, so lets fix all the timeouts.
One exception was the aux busy poll, where the poll sleep was much
longer than necessary (or optimal).
I measured the times on my setup, and now the sleep times are set to
such values that they result in multiple loops, but not too many (say,
5-10 loops). The timeouts were all increased to 100ms, which should be
more than enough for all of these, but in case of bad errors, shouldn't
stop the driver as multi-second timeouts could do.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Fixes: aa92213f38 ("drm/bridge: tc358767: Simplify polling in tc_link_training()")
Tested-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191209082707.24531-1-tomi.valkeinen@ti.com
Thierry needs fd70c7755b ("drm/bridge: tc358767: fix max_tu_symbol
value") to be able to merge his dp_link patch series.
Some adjacent changes conflicts, plus some clashes in i915 due to
cherry-picking and git trying to be helpful and leaving both versions
in.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
max_tu_symbol was programmed to TU_SIZE_RECOMMENDED - 1, which is not
what the spec says. The spec says:
roundup ((input active video bandwidth in bytes/output active video
bandwidth in bytes) * tu_size)
It is not quite clear what the above means, but calculating
max_tu_symbol = (input Bps / output Bps) * tu_size seems to work and
fixes the issues seen.
This fixes artifacts in some videomodes (e.g. 1024x768@60 on 2-lanes &
1.62Gbps was pretty bad for me).
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Tested-by: Jyri Sarha <jsarha@ti.com>
Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190924131702.9988-1-tomi.valkeinen@ti.com
Maarten needs -rc4 backmerged so he can pull in the fbcon notifier
removal topic branch into drm-misc-next.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
