[PATCH RESEND v5] drm/mediatek: dpi/dsi: fix possible_crtcs calculation
mtk_find_possible_crtcs() assumes that the main path will always have
the CRTC with id 0, the ext id 1 and the third id 2. This is only true
if the paths are all available. But paths are optional (see also
comment in mtk_drm_kms_init()), e.g. the main path might not be enabled
or available at all. Then the CRTC IDs will shift one up, e.g. ext will
be 0 and the third path will be 1.
To fix that, dynamically calculate the IDs by the presence of the paths.
While at it, make the return code a signed one and return -ENODEV if no
path is found and handle the error in the callers.
Fixes: 5aa8e7647676 ("drm/mediatek: dpi/dsi: Change the getting possible_crtc
way")
Suggested-by: Nícolas F. R. A. Prado
Signed-off-by: Michael Walle
---
You can find v4 at [1]. Unfortunately, it was never applied and in the
meantime there was a change in mtk_find_possible_crtcs(). So I've
dropped Nícolas Reviewed and Tested-by tags and Angelos Reviewed-by
tag.
[1] https://lore.kernel.org/r/20230905084922.3908121-2-mwa...@kernel.org/
---
drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 105
drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 2 +-
drivers/gpu/drm/mediatek/mtk_dpi.c | 5 +-
drivers/gpu/drm/mediatek/mtk_dsi.c | 5 +-
4 files changed, 78 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
index 17b036411292..9a8c1cace8a0 100644
--- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
@@ -514,29 +514,42 @@ static bool mtk_ddp_comp_find(struct device *dev,
return false;
}
-static unsigned int mtk_ddp_comp_find_in_route(struct device *dev,
- const struct mtk_drm_route
*routes,
- unsigned int num_routes,
- struct mtk_ddp_comp *ddp_comp)
+static int mtk_ddp_comp_find_in_route(struct device *dev,
+ const struct mtk_drm_route *routes,
+ unsigned int num_routes,
+ struct mtk_ddp_comp *ddp_comp)
{
- int ret;
unsigned int i;
- if (!routes) {
- ret = -EINVAL;
- goto err;
- }
+ if (!routes)
+ return -EINVAL;
for (i = 0; i < num_routes; i++)
if (dev == ddp_comp[routes[i].route_ddp].dev)
return BIT(routes[i].crtc_id);
- ret = -ENODEV;
-err:
+ return -ENODEV;
+}
- DRM_INFO("Failed to find comp in ddp table, ret = %d\n", ret);
+static bool mtk_ddp_path_available(const unsigned int *path,
+ unsigned int path_len,
+ struct device_node **comp_node)
+{
+ unsigned int i;
- return 0;
+ if (!path || !path_len)
+ return false;
+
+ for (i = 0U; i < path_len; i++) {
+ /* OVL_ADAPTOR doesn't have a device node */
+ if (path[i] == DDP_COMPONENT_DRM_OVL_ADAPTOR)
+ continue;
+
+ if (!comp_node[path[i]])
+ return false;
+ }
+
+ return true;
}
int mtk_ddp_comp_get_id(struct device_node *node,
@@ -554,32 +567,52 @@ int mtk_ddp_comp_get_id(struct device_node *node,
return -EINVAL;
}
-unsigned int mtk_find_possible_crtcs(struct drm_device *drm, struct device
*dev)
+int mtk_find_possible_crtcs(struct drm_device *drm, struct device *dev)
{
struct mtk_drm_private *private = drm->dev_private;
- unsigned int ret = 0;
-
- if (mtk_ddp_comp_find(dev,
- private->data->main_path,
- private->data->main_len,
- private->ddp_comp))
- ret = BIT(0);
- else if (mtk_ddp_comp_find(dev,
- private->data->ext_path,
- private->data->ext_len,
- private->ddp_comp))
- ret = BIT(1);
- else if (mtk_ddp_comp_find(dev,
- private->data->third_path,
- private->data->third_len,
- private->ddp_comp))
- ret = BIT(2);
- else
- ret = mtk_ddp_comp_find_in_route(dev,
-private->data->conn_routes,
-private->data->num_conn_routes,
-private->ddp_comp);
+ const struct mtk_mmsys_driver_data *data;
+ struct mtk_drm_private *priv_n;
+ int i = 0, j;
+ int ret;
+ for (j = 0; j < private->data->mmsys_dev_num
Re: [PATCH 01/20] drm/bridge: add dsi_lp11_notify mechanism
[+ Marek ]
Hi Dmitry,
> > > Some bridges have very strict power-up reqirements. In this case, the
> > > Toshiba TC358775. The reset has to be deasserted while *both* the DSI
> > > clock and DSI data lanes are in LP-11 mode. After the reset is relased,
> > > the bridge needs the DSI clock to actually be able to process I2C
> > > access. This access will configure the DSI side of the bridge during
> > > which the DSI data lanes have to be in LP-11 mode.
> >
> > Apparently this is an issue for a lot of DSI bridges. But enabling LP-11 for
> > a bridge is impossible with current documentation [1], which states "A DSI
> > host should keep the PHY powered down until the pre_enable operation is
> > called."
> > Additionally tc358767/tc9595 (DSI-DP bridge) needs LP-11 for AUX channel
> > access to even get EDID. This is a requirement before pre_enable would
> > even be possible.
> >
> > So some changes to the current flow are needed. But I am not so sure
> > about LP-11 notification. IMHO a device request to the DSI host to
> > enable LP-11 seems more sensible.
>
> Granted that there can be several DSI devices sharing the DSI bus (aka
> split-link), I was toying with the idea of making the DSI host call
> attached DSI devices when the transition happens.
So almost the same, as this patch?
> I don't have a fully working PoC and I probably won't have it ready til
> the end of May because of the lack of time and different local
> priorities.
Any news regarding this?
-michael
> > Best regards,
> > Alexander
> >
> > [1]
> > https://www.kernel.org/doc/html/latest/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
> >
> > > After everything is
> > > configured the video stream can finally be enabled.
> > >
> > > This means:
> > > (1) The bridge has to be configured completely in .pre_enable() op
> > > (with the clock turned on and data lanes in LP-11 mode, thus
> > > .pre_enable_prev_first has to be set).
> > > (2) The bridge will enable its output in the .enable() op
> > > (3) There must be some mechanism before (1) where the bridge can
> > > release its reset while the clock lane is still in LP-11 mode.
> > >
> > > Unfortunately, (3) is crucial for a correct operation of the bridge.
> > > To satisfy this requriment, introduce a new callback .dsi_lp11_notify()
> > > which will be called by the DSI host driver.
> > >
> > > Signed-off-by: Michael Walle
> > > ---
> > > drivers/gpu/drm/drm_bridge.c | 16
> > > include/drm/drm_bridge.h | 12
> > > 2 files changed, 28 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > > index 28abe9aa99ca..98cd6558aecb 100644
> > > --- a/drivers/gpu/drm/drm_bridge.c
> > > +++ b/drivers/gpu/drm/drm_bridge.c
> > > @@ -1339,6 +1339,22 @@ void drm_bridge_hpd_notify(struct drm_bridge
> > > *bridge,
> > > }
> > > EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify);
> > >
> > > +/**
> > > + * drm_bridge_dsi_lp11_notify - notify clock/data lanes LP-11 mode
> > > + * @bridge: bridge control structure
> > > + *
> > > + * DSI host drivers shall call this function while the clock and data
> > > lanes
> > > + * are still in LP-11 mode.
> > > + *
> > > + * This function shall be called in a context that can sleep.
> > > + */
> > > +void drm_bridge_dsi_lp11_notify(struct drm_bridge *bridge)
> > > +{
> > > + if (bridge->funcs->dsi_lp11_notify)
> > > + bridge->funcs->dsi_lp11_notify(bridge);
> > > +}
> > > +EXPORT_SYMBOL_GPL(drm_bridge_dsi_lp11_notify);
> > > +
> > > #ifdef CONFIG_OF
> > > /**
> > > * of_drm_find_bridge - find the bridge corresponding to the device node
> > > in
> > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > > index 4baca0d9107b..4ef61274e0a8 100644
> > > --- a/include/drm/drm_bridge.h
> > > +++ b/include/drm/drm_bridge.h
> > > @@ -630,6 +630,17 @@ struct drm_bridge_funcs {
> > >*/
> > > void (*hpd_disable)(struct drm_bridge *bridge);
> > >
> > > + /**
> > > + * dsi_lp11_notify:
> > > + *
> > > + * Will be called by the DSI host driver while both the DSI clock
> > > + * lane as well as the DSI data lanes are in LP-11 mode. Some bri
Re: [PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup
On Mon May 6, 2024 at 3:34 PM CEST, Michael Walle wrote: > This patchset fixes the bridge initialization according to the > datasheet. Not sure how that even worked before. Maybe because the > initialization was done prior to linux (?). > > The bridge has some peculiarities: > (1) The reset has to be deasserted in LP-11 mode > (2) For I2C access the bridge needs the DSI clock > (3) The bridge has to be configured while the video stream is > disabled. > (4) The bridge has limitations on the display timings. In particular, > the horizontal pulse width has to be at least 8 pixels wide and > both the horizontal pulse width as well as the back porch has to > be even. According to the datasheet the horizontal front porch as > well but in line sync mode, this is ignored. Also line sync is the > only supported mode for this bridge, therefore, the front porch > is always ignored. > > The most controversial patch is probably "drm/bridge: add > dsi_lp11_notify mechanism" which is needed for (1) above. Some time ago > there was a series [1] to add a manual power-up, which was abandoned and > which didn't suite the needs for this bridge anyway. > > Also, this will gradually change the tc_ prefix to tc358775_ while the > functions are refactored. > > The bridge was successfully tested on a Mediatek MT8195 SoC with the > following panels: > - Innolux G101ICE > - AUO G121EAN01.0 > - Innolux G156HCE (dual-link LVDS) > > [1] > https://lore.kernel.org/r/20231016165355.1327217-1-dmitry.barysh...@linaro.org/ Any comments on this series, besides the discussion on how to do the reset during LP11? Most of the other patches should be more or less self contained. -michael signature.asc Description: PGP signature
Re: [PATCH v4 3/3] drm/mediatek: Implement OF graphs support for display paths
Hi Angelo, > >> Implement OF graphs support to the mediatek-drm drivers, allowing to > >> stop hardcoding the paths, and preventing this driver to get a huge > >> amount of arrays for each board and SoC combination, also paving the > >> way to share the same mtk_mmsys_driver_data between multiple SoCs, > >> making it more straightforward to add support for new chips. > > > > paths might be optional, see comment in mtk_drm_kms_init(). But with > > this patch, you'll get an -EINVAL with a disabled path. See my > > proposals how to fix that below. > > I might not be understanding the reason behind allowing that but, per my > logic, if > a board does have a path, then it's written in devicetree and enabled - > otherwise, > it should not be there at all, in principle. > > > Can you explain a bit more extensively the reason(s) why we need to account > for disabled paths? Paths should be (and this was already supported before this patch with the hardcoded paths) disabled with the status property. This way you can have a common board configuration where all the paths are already described but are disabled. An overlay (or maybe another dts variant) can then just enable the pipeline/output port by overwriting the status property. Also, this is the usual DT usage, as a node with status = "disabled" should just be skipped. Without handling this, the current code will return -EINVAL during probe (IIRC, my vacation might have reset my memory :o). -michael signature.asc Description: PGP signature
Re: [PATCH v4 3/3] drm/mediatek: Implement OF graphs support for display paths
Hi Angelo,
On Thu May 16, 2024 at 10:11 AM CEST, AngeloGioacchino Del Regno wrote:
> Implement OF graphs support to the mediatek-drm drivers, allowing to
> stop hardcoding the paths, and preventing this driver to get a huge
> amount of arrays for each board and SoC combination, also paving the
> way to share the same mtk_mmsys_driver_data between multiple SoCs,
> making it more straightforward to add support for new chips.
paths might be optional, see comment in mtk_drm_kms_init(). But with
this patch, you'll get an -EINVAL with a disabled path. See my
proposals how to fix that below.
With these changes and the following two patches I was able to get
DisplayPort working on vdosys1. vdosys0 wasn't used at all.
https://lore.kernel.org/r/20240516145824.1669263-1-mwa...@kernel.org/
https://lore.kernel.org/r/20240517093024.1702750-1-mwa...@kernel.org/
I've already successfully tested a former version with DSI output on
vdosys0.
Thanks for working on this!
> +/**
> + * mtk_drm_of_ddp_path_build_one - Build a Display HW Pipeline for a CRTC
> Path
> + * @dev: The mediatek-drm device
> + * @cpath:CRTC Path relative to a VDO or MMSYS
> + * @out_path: Pointer to an array that will contain the new pipeline
> + * @out_path_len: Number of entries in the pipeline array
> + *
> + * MediaTek SoCs can use different DDP hardware pipelines (or paths)
> depending
> + * on the board-specific desired display configuration; this function walks
> + * through all of the output endpoints starting from a VDO or MMSYS hardware
> + * instance and builds the right pipeline as specified in device trees.
> + *
> + * Return:
> + * * %0 - Display HW Pipeline successfully built and validated
> + * * %-ENOENT - Display pipeline was not specified in device tree
> + * * %-EINVAL - Display pipeline built but validation failed
> + * * %-ENOMEM - Failure to allocate pipeline array to pass to the caller
> + */
> +static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum
> mtk_crtc_path cpath,
> + const unsigned int **out_path,
> + unsigned int *out_path_len)
> +{
> + struct device_node *next, *prev, *vdo = dev->parent->of_node;
> + unsigned int temp_path[DDP_COMPONENT_DRM_ID_MAX] = { 0 };
> + unsigned int *final_ddp_path;
> + unsigned short int idx = 0;
> + bool ovl_adaptor_comp_added = false;
> + int ret;
> +
> + /* Get the first entry for the temp_path array */
> + ret = mtk_drm_of_get_ddp_ep_cid(vdo, 0, cpath, , _path[idx]);
> + if (ret) {
> + if (next && temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) {
> + dev_err(dev, "Adding OVL Adaptor for %pOF\n", next);
> + ovl_adaptor_comp_added = true;
> + } else {
> + if (next)
> + dev_err(dev, "Invalid component %pOF\n", next);
> + else
> + dev_err(dev, "Cannot find first endpoint for
> path %d\n", cpath);
> +
> + return ret;
> + }
> + }
> + idx++;
> +
> + /*
> + * Walk through port outputs until we reach the last valid mediatek-drm
> component.
> + * To be valid, this must end with an "invalid" component that is a
> display node.
> + */
> + do {
> + prev = next;
> + ret = mtk_drm_of_get_ddp_ep_cid(next, 1, cpath, ,
> _path[idx]);
> + of_node_put(prev);
> + if (ret) {
> + of_node_put(next);
> + break;
> + }
> +
> + /*
> + * If this is an OVL adaptor exclusive component and one of
> those
> + * was already added, don't add another instance of the generic
> + * DDP_COMPONENT_OVL_ADAPTOR, as this is used only to decide
> whether
> + * to probe that component master driver of which only one
> instance
> + * is needed and possible.
> + */
> + if (temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) {
> + if (!ovl_adaptor_comp_added)
> + ovl_adaptor_comp_added = true;
> + else
> + idx--;
> + }
> + } while (++idx < DDP_COMPONENT_DRM_ID_MAX);
/* The device might not be disabled. In that case, don't check the last
* entry but just report the missing device. */
if (ret == -ENODEV)
return ret;
> +
> + /* If the last entry is not a final display output, the configuration
> is wrong */
> + switch (temp_path[idx - 1]) {
> + case DDP_COMPONENT_DP_INTF0:
> + case DDP_COMPONENT_DP_INTF1:
> + case DDP_COMPONENT_DPI0:
> + case DDP_COMPONENT_DPI1:
> + case DDP_COMPONENT_DSI0:
> + case DDP_COMPONENT_DSI1:
> + case DDP_COMPONENT_DSI2:
> + case DDP_COMPONENT_DSI3:
> + break;
> +
[PATCH v5] drm/mediatek: dpi/dsi: fix possible_crtcs calculation
mtk_find_possible_crtcs() assumes that the main path will always have
the CRTC with id 0, the ext id 1 and the third id 2. This is only true
if the paths are all available. But paths are optional (see also
comment in mtk_drm_kms_init()), e.g. the main path might not be enabled
or available at all. Then the CRTC IDs will shift one up, e.g. ext will
be 0 and the third path will be 1.
To fix that, dynamically calculate the IDs by the presence of the paths.
While at it, make the return code a signed one and return -ENODEV if no
path is found and handle the error in the callers.
Fixes: 5aa8e7647676 ("drm/mediatek: dpi/dsi: Change the getting possible_crtc
way")
Suggested-by: Nícolas F. R. A. Prado
Signed-off-by: Michael Walle
---
You can find v4 at [1]. Unfortunately, it was never applied and in the
meantime there was a change in mtk_find_possible_crtcs(). So I've
dropped Nícolas Reviewed and Tested-by tags and Angelos Reviewed-by
tag.
[1] https://lore.kernel.org/r/20230905084922.3908121-2-mwa...@kernel.org/
---
drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 105
drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 2 +-
drivers/gpu/drm/mediatek/mtk_dpi.c | 5 +-
drivers/gpu/drm/mediatek/mtk_dsi.c | 5 +-
4 files changed, 78 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
index 17b036411292..9a8c1cace8a0 100644
--- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
@@ -514,29 +514,42 @@ static bool mtk_ddp_comp_find(struct device *dev,
return false;
}
-static unsigned int mtk_ddp_comp_find_in_route(struct device *dev,
- const struct mtk_drm_route
*routes,
- unsigned int num_routes,
- struct mtk_ddp_comp *ddp_comp)
+static int mtk_ddp_comp_find_in_route(struct device *dev,
+ const struct mtk_drm_route *routes,
+ unsigned int num_routes,
+ struct mtk_ddp_comp *ddp_comp)
{
- int ret;
unsigned int i;
- if (!routes) {
- ret = -EINVAL;
- goto err;
- }
+ if (!routes)
+ return -EINVAL;
for (i = 0; i < num_routes; i++)
if (dev == ddp_comp[routes[i].route_ddp].dev)
return BIT(routes[i].crtc_id);
- ret = -ENODEV;
-err:
+ return -ENODEV;
+}
- DRM_INFO("Failed to find comp in ddp table, ret = %d\n", ret);
+static bool mtk_ddp_path_available(const unsigned int *path,
+ unsigned int path_len,
+ struct device_node **comp_node)
+{
+ unsigned int i;
- return 0;
+ if (!path || !path_len)
+ return false;
+
+ for (i = 0U; i < path_len; i++) {
+ /* OVL_ADAPTOR doesn't have a device node */
+ if (path[i] == DDP_COMPONENT_DRM_OVL_ADAPTOR)
+ continue;
+
+ if (!comp_node[path[i]])
+ return false;
+ }
+
+ return true;
}
int mtk_ddp_comp_get_id(struct device_node *node,
@@ -554,32 +567,52 @@ int mtk_ddp_comp_get_id(struct device_node *node,
return -EINVAL;
}
-unsigned int mtk_find_possible_crtcs(struct drm_device *drm, struct device
*dev)
+int mtk_find_possible_crtcs(struct drm_device *drm, struct device *dev)
{
struct mtk_drm_private *private = drm->dev_private;
- unsigned int ret = 0;
-
- if (mtk_ddp_comp_find(dev,
- private->data->main_path,
- private->data->main_len,
- private->ddp_comp))
- ret = BIT(0);
- else if (mtk_ddp_comp_find(dev,
- private->data->ext_path,
- private->data->ext_len,
- private->ddp_comp))
- ret = BIT(1);
- else if (mtk_ddp_comp_find(dev,
- private->data->third_path,
- private->data->third_len,
- private->ddp_comp))
- ret = BIT(2);
- else
- ret = mtk_ddp_comp_find_in_route(dev,
-private->data->conn_routes,
-private->data->num_conn_routes,
-private->ddp_comp);
+ const struct mtk_mmsys_driver_data *data;
+ struct mtk_drm_private *priv_n;
+ int i = 0, j;
+ int ret;
+ for (j = 0; j < private->data->mmsys_dev_num
[PATCH 20/20] drm/bridge: tc358775: use devm_drm_bridge_add()
Use the device resource managed version of drm_bridge_add(). This
simplifies the error handling and we can get rid of tc_remove_bridge().
Also, add a check for the return code.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 21 -
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index 31f89b7d5a3a..1d2547e4c4e3 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -762,26 +762,14 @@ static int tc_probe(struct i2c_client *client)
tc->bridge.funcs = _bridge_funcs;
tc->bridge.of_node = dev->of_node;
tc->bridge.pre_enable_prev_first = true;
- drm_bridge_add(>bridge);
- i2c_set_clientdata(client, tc);
-
- ret = tc_attach_host(tc);
+ ret = devm_drm_bridge_add(tc->dev, >bridge);
if (ret)
- goto err_bridge_remove;
-
- return 0;
-
-err_bridge_remove:
- drm_bridge_remove(>bridge);
- return ret;
-}
+ return ret;
-static void tc_remove(struct i2c_client *client)
-{
- struct tc_data *tc = i2c_get_clientdata(client);
+ i2c_set_clientdata(client, tc);
- drm_bridge_remove(>bridge);
+ return tc_attach_host(tc);
}
static const struct i2c_device_id tc358775_i2c_ids[] = {
@@ -805,7 +793,6 @@ static struct i2c_driver tc358775_driver = {
},
.id_table = tc358775_i2c_ids,
.probe = tc_probe,
- .remove = tc_remove,
};
module_i2c_driver(tc358775_driver);
--
2.39.2
[PATCH 19/20] drm/bridge: tc358775: fix power-up sequencing
The reset line of this bridge must be released while the DSI data lanes
and DSI clock lane are in LP-11 mode. After that the DSI clock has to be
turned on, which is a requirement to have I2C work.
To achieve this, use the new .lp11_notify() callback where the reset
line is released. Set .pre_enable_prev_first to make sure, there is a
valid DSI clock during the .pre_enabe() op. In .pre_enable() the bridge
will be fully configured but the LVDS transmitter will remain disabled.
It will eventually be enabled in the .enable() op.
With the correct initialization sequence we don't need the additional
reset, nor the additional write to VFUEN. With that fixed, the init
sequence is exactly how the vendor is requiring it.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 62 +++
1 file changed, 37 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index 99dbbb1fee78..31f89b7d5a3a 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -239,6 +239,9 @@ static void tc358775_power_up(struct tc_data *tc)
struct device *dev = >dsi->dev;
int ret;
+ if (tc->powered)
+ return;
+
ret = regulator_enable(tc->vddio);
if (ret < 0)
dev_err(dev, "regulator vddio enable failed, %d\n", ret);
@@ -252,6 +255,8 @@ static void tc358775_power_up(struct tc_data *tc)
gpiod_set_value(tc->reset_gpio, 0);
usleep_range(200, 250);
+
+ tc->powered = true;
}
static void tc358775_power_down(struct tc_data *tc)
@@ -271,20 +276,8 @@ static void tc358775_power_down(struct tc_data *tc)
ret = regulator_disable(tc->vddio);
if (ret < 0)
dev_err(dev, "regulator vddio disable failed, %d\n", ret);
-}
-static void tc_bridge_pre_enable(struct drm_bridge *bridge)
-{
- struct tc_data *tc = bridge_to_tc(bridge);
-
- tc358775_power_up(tc);
-}
-
-static void tc_bridge_post_disable(struct drm_bridge *bridge)
-{
- struct tc_data *tc = bridge_to_tc(bridge);
-
- tc358775_power_down(tc);
+ tc->powered = false;
}
/* helper function to access bus_formats */
@@ -474,12 +467,25 @@ static void tc358775_configure_lvds_clock(struct tc_data
*tc)
regmap_write(tc->regmap, LVCFG, val);
}
-static void tc358775_bridge_enable(struct drm_bridge *bridge)
+static void tc358775_dsi_lp11_notify(struct drm_bridge *bridge)
{
struct tc_data *tc = bridge_to_tc(bridge);
- unsigned int val = 0;
- struct drm_display_mode *mode;
+
+ tc358775_power_up(tc);
+}
+
+static void tc358775_bridge_pre_enable(struct drm_bridge *bridge)
+{
struct drm_connector *connector = get_connector(bridge->encoder);
+ struct tc_data *tc = bridge_to_tc(bridge);
+ struct drm_display_mode *mode;
+ unsigned int val = 0;
+
+ /*
+* Legacy behavior, make sure this bridge is powered even if
+* drm_bridge_dsi_lp11_notify() isn't called by the DSI host
+*/
+ tc358775_power_up(tc);
mode = >encoder->crtc->state->adjusted_mode;
@@ -488,22 +494,27 @@ static void tc358775_bridge_enable(struct drm_bridge
*bridge)
dev_info(tc->dev, "DSI2LVDS Chip ID.%02x Revision ID. %02x **\n",
(val >> 8) & 0xFF, val & 0xFF);
- regmap_write(tc->regmap, SYSRST,
-SYS_RST_REG | SYS_RST_DSIRX | SYS_RST_BM | SYS_RST_LCD |
-SYS_RST_I2CM);
- usleep_range(3, 4);
-
tc358775_configure_dsi(tc, mode->crtc_clock);
tc358775_configure_lvds_timings(tc, mode);
tc358775_configure_pll(tc, mode->crtc_clock);
tc358775_configure_color_mapping(tc,
connector->display_info.bus_formats[0]);
- regmap_write(tc->regmap, VFUEN, VFUEN_VFUEN);
tc358775_configure_lvds_clock(tc);
+}
+
+static void tc358775_bridge_enable(struct drm_bridge *bridge)
+{
+ struct tc_data *tc = bridge_to_tc(bridge);
- /* Finally, enable the LVDS transmitter */
regmap_update_bits(tc->regmap, LVCFG, LVCFG_LVEN, LVCFG_LVEN);
}
+static void tc358775_bridge_post_disable(struct drm_bridge *bridge)
+{
+ struct tc_data *tc = bridge_to_tc(bridge);
+
+ tc358775_power_down(tc);
+}
+
/*
* According to the datasheet, the horizontal back porch, front porch and sync
* length must be a multiple of 2 and the minimal horizontal pulse width is 8.
@@ -634,11 +645,12 @@ static int tc_bridge_attach(struct drm_bridge *bridge,
static const struct drm_bridge_funcs tc_bridge_funcs = {
.attach = tc_bridge_attach,
- .pre_enable = tc_bridge_pre_enable,
+ .dsi_lp11_notify = tc358775_dsi_lp11_notify,
+ .pre_enable = tc358775_bridge_pre_enable,
.enable = tc358775_bridge_enabl
[PATCH 18/20] drm/bridge: tc358775: fix the power-up/down delays
Implement the delays according to Figure 8-10 and 8-11 of the datasheet. In particular, the datasheet states that the *maximum* time between enabling the VDDIO and VDD is 10ms. Currently, as implemented this is always violated. Of course, this is only a best effort because we cannot be sure enabling of the two regulators will be that fast. The time between releasing the stby GPIO and releasing the reset GPIO must be at least 10us and not 10ms as it was before this patch. After reset is released, there must be at least a delay of 200us until the first HS clock is received. Signed-off-by: Michael Walle --- drivers/gpu/drm/bridge/tc358775.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358775.c b/drivers/gpu/drm/bridge/tc358775.c index d5b3d691d2c1..99dbbb1fee78 100644 --- a/drivers/gpu/drm/bridge/tc358775.c +++ b/drivers/gpu/drm/bridge/tc358775.c @@ -242,18 +242,16 @@ static void tc358775_power_up(struct tc_data *tc) ret = regulator_enable(tc->vddio); if (ret < 0) dev_err(dev, "regulator vddio enable failed, %d\n", ret); - usleep_range(1, 11000); ret = regulator_enable(tc->vdd); if (ret < 0) dev_err(dev, "regulator vdd enable failed, %d\n", ret); - usleep_range(1, 11000); gpiod_set_value(tc->stby_gpio, 0); - usleep_range(1, 11000); + usleep_range(10, 20); gpiod_set_value(tc->reset_gpio, 0); - usleep_range(10, 20); + usleep_range(200, 250); } static void tc358775_power_down(struct tc_data *tc) @@ -265,17 +263,14 @@ static void tc358775_power_down(struct tc_data *tc) usleep_range(10, 20); gpiod_set_value(tc->stby_gpio, 1); - usleep_range(1, 11000); ret = regulator_disable(tc->vdd); if (ret < 0) dev_err(dev, "regulator vdd disable failed, %d\n", ret); - usleep_range(1, 11000); ret = regulator_disable(tc->vddio); if (ret < 0) dev_err(dev, "regulator vddio disable failed, %d\n", ret); - usleep_range(1, 11000); } static void tc_bridge_pre_enable(struct drm_bridge *bridge) -- 2.39.2
[PATCH 17/20] drm/bridge: tc358775: move bridge power up/down into functions
Move the bridge power-up and power-down handling into own functions.
This is a preparation patch to fix the power-up sequencing of the
bridge. No functional change.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 21 +
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index c50554ec4b28..d5b3d691d2c1 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -215,6 +215,7 @@ struct tc_data {
struct gpio_desc*reset_gpio;
struct gpio_desc*stby_gpio;
boollvds_dual_link;
+ boolpowered;
u8 bpc;
enum tc3587x5_type type;
@@ -233,9 +234,8 @@ static inline struct tc_data *bridge_to_tc(struct
drm_bridge *b)
return container_of(b, struct tc_data, bridge);
}
-static void tc_bridge_pre_enable(struct drm_bridge *bridge)
+static void tc358775_power_up(struct tc_data *tc)
{
- struct tc_data *tc = bridge_to_tc(bridge);
struct device *dev = >dsi->dev;
int ret;
@@ -256,9 +256,8 @@ static void tc_bridge_pre_enable(struct drm_bridge *bridge)
usleep_range(10, 20);
}
-static void tc_bridge_post_disable(struct drm_bridge *bridge)
+static void tc358775_power_down(struct tc_data *tc)
{
- struct tc_data *tc = bridge_to_tc(bridge);
struct device *dev = >dsi->dev;
int ret;
@@ -279,6 +278,20 @@ static void tc_bridge_post_disable(struct drm_bridge
*bridge)
usleep_range(1, 11000);
}
+static void tc_bridge_pre_enable(struct drm_bridge *bridge)
+{
+ struct tc_data *tc = bridge_to_tc(bridge);
+
+ tc358775_power_up(tc);
+}
+
+static void tc_bridge_post_disable(struct drm_bridge *bridge)
+{
+ struct tc_data *tc = bridge_to_tc(bridge);
+
+ tc358775_power_down(tc);
+}
+
/* helper function to access bus_formats */
static struct drm_connector *get_connector(struct drm_encoder *encoder)
{
--
2.39.2
[PATCH 16/20] drm/bridge: tc358775: use proper defines to configure LVDS timings
Provide bitfield macros for the individual fields in the LVDS timing
registers and get rid of the magic values.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 52 +--
1 file changed, 33 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index 33a97ddba7b5..c50554ec4b28 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -111,11 +111,19 @@
#define VPCTRL_OPXLFMT BIT(8)
#define VPCTRL_EVTMODE BIT(5) /* Video event mode enable, tc35876x only */
#define HTIM1 0x0454 /* Horizontal Timing Control 1 */
+#define HTIM1_HPW GENMASK(8, 0)
+#define HTIM1_HBPR GENMASK(24, 16)
#define HTIM2 0x0458 /* Horizontal Timing Control 2 */
+#define HTIM2_HACT GENMASK(10, 0)
+#define HTIM2_HFPR GENMASK(24, 16)
#define VTIM1 0x045C /* Vertical Timing Control 1 */
+#define VTIM1_VPW GENMASK(7, 0)
+#define VTIM1_VBPR GENMASK(23, 16)
#define VTIM2 0x0460 /* Vertical Timing Control 2 */
+#define VTIM2_VACT GENMASK(10, 0)
+#define VTIM2_VFPR GENMASK(23, 16)
#define VFUEN 0x0464 /* Video Frame Timing Update Enable */
-#define VFUEN_EN BIT(0) /* Upload Enable */
+#define VFUEN_VFUENBIT(0) /* Upload Enable */
/* Mux Input Select for LVDS LINK Input */
#define LV_MX00030x0480 /* Bit 0 to 3 */
@@ -346,24 +354,19 @@ static void tc358775_configure_dsi(struct tc_data *tc,
unsigned int pixelclk)
static void tc358775_configure_lvds_timings(struct tc_data *tc,
struct drm_display_mode *mode)
{
- u32 hback_porch, hsync_len, hfront_porch, hactive, htime1, htime2;
- u32 vback_porch, vsync_len, vfront_porch, vactive, vtime1, vtime2;
+ u32 hback_porch, hsync_len, hfront_porch, hactive;
+ u32 vback_porch, vsync_len, vfront_porch, vactive;
+ unsigned int val;
hback_porch = mode->htotal - mode->hsync_end;
hsync_len = mode->hsync_end - mode->hsync_start;
+ hactive = mode->hdisplay;
+ hfront_porch = mode->hsync_start - mode->hdisplay;
+
vback_porch = mode->vtotal - mode->vsync_end;
vsync_len = mode->vsync_end - mode->vsync_start;
-
- htime1 = (hback_porch << 16) + hsync_len;
- vtime1 = (vback_porch << 16) + vsync_len;
-
- hfront_porch = mode->hsync_start - mode->hdisplay;
- hactive = mode->hdisplay;
- vfront_porch = mode->vsync_start - mode->vdisplay;
vactive = mode->vdisplay;
-
- htime2 = (hfront_porch << 16) + hactive;
- vtime2 = (vfront_porch << 16) + vactive;
+ vfront_porch = mode->vsync_start - mode->vdisplay;
/* Video event mode vs pulse mode bit, does not exist for tc358775 */
if (tc->type == TC358765)
@@ -379,12 +382,23 @@ static void tc358775_configure_lvds_timings(struct
tc_data *tc,
regmap_update_bits(tc->regmap, VPCTRL, val,
VPCTRL_OPXLFMT | VPCTRL_MSF | VPCTRL_EVTMODE);
- regmap_write(tc->regmap, HTIM1, htime1);
- regmap_write(tc->regmap, VTIM1, vtime1);
- regmap_write(tc->regmap, HTIM2, htime2);
- regmap_write(tc->regmap, VTIM2, vtime2);
+ val = u32_encode_bits(hsync_len, HTIM1_HPW);
+ val |= u32_encode_bits(hback_porch, HTIM1_HBPR);
+ regmap_write(tc->regmap, HTIM1, val);
+
+ val = u32_encode_bits(hactive, HTIM2_HACT);
+ val |= u32_encode_bits(hfront_porch, HTIM2_HFPR);
+ regmap_write(tc->regmap, HTIM2, val);
+
+ val = u32_encode_bits(vsync_len, VTIM1_VPW);
+ val |= u32_encode_bits(vback_porch, VTIM1_VBPR);
+ regmap_write(tc->regmap, VTIM1, val);
+
+ val = u32_encode_bits(vactive, VTIM2_VACT);
+ val |= u32_encode_bits(vfront_porch, VTIM2_VFPR);
+ regmap_write(tc->regmap, VTIM2, val);
- regmap_write(tc->regmap, VFUEN, VFUEN_EN);
+ regmap_write(tc->regmap, VFUEN, VFUEN_VFUEN);
}
static const struct tc358775_pll_settings tc358775_pll_settings[] = {
@@ -475,7 +489,7 @@ static void tc358775_bridge_enable(struct drm_bridge
*bridge)
tc358775_configure_lvds_timings(tc, mode);
tc358775_configure_pll(tc, mode->crtc_clock);
tc358775_configure_color_mapping(tc,
connector->display_info.bus_formats[0]);
- regmap_write(tc->regmap, VFUEN, VFUEN_EN);
+ regmap_write(tc->regmap, VFUEN, VFUEN_VFUEN);
tc358775_configure_lvds_clock(tc);
/* Finally, enable the LVDS transmitter */
--
2.39.2
[PATCH 15/20] drm/bridge: tc358775: dynamically configure DSI link settings
Instead of hardcoding the settings for just one (unknown) particular
frequency and lane setting, compute the DSI link parameters using the
handy phy_mipi_dphy_get_default_config() helper function.
The DSI_START and DSI_BUSY registers were removed in version 0.6 of the
datasheet. It seems that it applies to a different bridge and was just a
leftover. Remove the DSI_START handling and the (unused) DSI_BUSY macro.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/Kconfig| 1 +
drivers/gpu/drm/bridge/tc358775.c | 58 +++
2 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index c621be1a99a8..ed018d6f1da3 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -349,6 +349,7 @@ config DRM_TOSHIBA_TC358775
select REGMAP_I2C
select DRM_PANEL
select DRM_MIPI_DSI
+ select GENERIC_PHY_MIPI_DPHY
help
Toshiba TC358775 DSI/LVDS bridge chip driver.
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index e3fba7ac71ec..33a97ddba7b5 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
@@ -49,12 +50,14 @@
/* DSI PPI Layer Registers */
#define PPI_STARTPPI0x0104 /* START control bit of PPI-TX function. */
-#define PPI_START_FUNCTION 1
+#define PPI_STARTPPI_STARTPPI BIT(0)
#define PPI_BUSYPPI 0x0108
#define PPI_LINEINITCNT 0x0110 /* Line Initialization Wait Counter */
#define PPI_LPTXTIMECNT 0x0114
#define PPI_LANEENABLE 0x0134 /* Enables each lane at the PPI layer. */
+#define LANEENABLE_CLENBIT(0)
+#define LANEENABLE_L0ENBIT(1)
#define PPI_TX_RX_TA0x013C /* DSI Bus Turn Around timing parameters */
/* Analog timer function enable */
@@ -89,10 +92,7 @@
#define PPI_CLRSIPO 0x01E4 /* Clear SIPO values, Slave mode use only. */
#define HSTIMEOUT 0x01F0 /* HS Rx Time Out Counter */
#define HSTIMEOUTENABLE 0x01F4 /* Enable HS Rx Time Out Counter */
-#define DSI_STARTDSI0x0204 /* START control bit of DSI-TX function */
-#define DSI_RX_START 1
-#define DSI_BUSYDSI 0x0208
#define DSI_LANEENABLE 0x0210 /* Enables each lane at the Protocol layer. */
#define DSI_LANESTATUS0 0x0214 /* Displays lane is in HS RX mode. */
#define DSI_LANESTATUS1 0x0218 /* Displays lane is in ULPS or STOP state */
@@ -174,21 +174,12 @@ enum {
/* Chip ID and Revision ID Register */
#define IDREG 0x0580
-#define LPX_PERIOD 4
-#define TTA_GET0x4
-#define TTA_SURE 6
-#define SINGLE_LINK1
-#define DUAL_LINK 2
-
#define TC358775XBG_ID 0x7500
/* Debug Registers */
#define DEBUG00 0x05A0 /* Debug */
#define DEBUG01 0x05A4 /* LVDS Data */
-#define DSI_CLEN_BIT BIT(0)
-#define L0EN BIT(1)
-
enum tc358775_ports {
TC358775_DSI_IN,
TC358775_LVDS_OUT0,
@@ -314,23 +305,42 @@ static const struct reg_sequence tc_lvmux_jeida18_24[] = {
{ LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R0) },
};
-static void tc358775_configure_dsi(struct tc_data *tc)
+/* All the DSI timing is counted by the HS byte clock internally */
+static uint32_t tc358775_ps_to_cnt(unsigned long long ps,
+ struct phy_configure_opts_mipi_dphy *cfg)
{
+ unsigned long hs_byte_clk = cfg->hs_clk_rate / 8;
+
+ return DIV_ROUND_UP(ps * hs_byte_clk, PSEC_PER_SEC);
+}
+
+static void tc358775_configure_dsi(struct tc_data *tc, unsigned int pixelclk)
+{
+ int bpp = mipi_dsi_pixel_format_to_bpp(tc->dsi->format);
+ struct phy_configure_opts_mipi_dphy cfg;
unsigned int val;
- regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
- regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
- regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 3);
- regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 3);
- regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 3);
- regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 3);
+ phy_mipi_dphy_get_default_config(pixelclk * 1000, bpp,
+tc->num_dsi_lanes, );
+
+ regmap_write(tc->regmap, PPI_TX_RX_TA,
+(tc358775_ps_to_cnt(cfg.ta_get, ) << 16) |
+tc358775_ps_to_cnt(cfg.ta_sure, ));
+ regmap_write(tc->regmap, PPI_LPTXTIMECNT,
+tc358775_ps_to_cnt(cfg.lpx, ));
+
+ val = tc358775_ps_to_cnt(cfg.hs_settle, );
+ regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, val);
+ regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, val);
+ regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, val);
+ regmap_write(tc->regmap, PPI_D3S_CLRSIPOCO
[PATCH 14/20] drm/bridge: tc358775: configure PLL depending on the LVDS clock
The PLL setting was hardcoded to a LVDS clock between 60MHz and 135MHz.
This adds support for slower frequencies. Also, rework the reset
sequence to match the initialization sequence provided by the vendor.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 50 ---
1 file changed, 42 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index 4ec059531c5f..e3fba7ac71ec 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -30,8 +30,6 @@
#include
#include
-#define FLD_VAL(val, start, end) FIELD_PREP(GENMASK(start, end), val)
-
/* Registers */
/* DSI D-PHY Layer Registers */
@@ -146,10 +144,10 @@ enum {
#define PCLKSEL_HSRCK 0 /* DSI clock */
#define LVPHY0 0x04A0 /* LVDS PHY 0 */
-#define LV_PHY0_RST(v) FLD_VAL(v, 22, 22) /* PHY reset */
-#define LV_PHY0_IS(v) FLD_VAL(v, 15, 14)
-#define LV_PHY0_ND(v) FLD_VAL(v, 4, 0) /* Frequency range select */
-#define LV_PHY0_PRBS_ON(v) FLD_VAL(v, 20, 16) /* Clock/Data Flag pins */
+#define LVPHY0_LV_ND GENMASK(4, 0)
+#define LVPHY0_LV_FS GENMASK(6, 5)
+#define LVPHY0_LV_IS GENMASK(15, 14) /* charge pump current */
+#define LVPHY0_LV_RST BIT(22)
#define LVPHY1 0x04A4 /* LVDS PHY 1 */
#define SYSSTAT 0x0500 /* System Status */
@@ -223,6 +221,14 @@ struct tc_data {
enum tc3587x5_type type;
};
+struct tc358775_pll_settings {
+ unsigned int min_khz;
+ unsigned int max_khz;
+ u8 fs;
+ u8 nd;
+ u8 is;
+};
+
static inline struct tc_data *bridge_to_tc(struct drm_bridge *b)
{
return container_of(b, struct tc_data, bridge);
@@ -371,10 +377,38 @@ static void tc358775_configure_lvds_timings(struct
tc_data *tc,
regmap_write(tc->regmap, VFUEN, VFUEN_EN);
}
-static void tc358775_configure_pll(struct tc_data *tc, int pixelclk)
+static const struct tc358775_pll_settings tc358775_pll_settings[] = {
+ { 25000, 3, 2, 27, 1 },
+ { 3, 6, 1, 13, 1 },
+ { 6, 135000, 0, 6, 1 },
+ {}
+};
+
+static void tc358775_configure_pll(struct tc_data *tc, unsigned int pixelclk)
{
+ const struct tc358775_pll_settings *settings;
+ unsigned int val;
+
+ if (tc->lvds_dual_link)
+ pixelclk /= 2;
+
+ for (settings = tc358775_pll_settings; settings->min_khz; settings++)
+ if (pixelclk > settings->min_khz &&
+ pixelclk < settings->max_khz)
+ break;
+
+ if (!settings->min_khz)
+ return;
+
+ val = u32_encode_bits(settings->fs, LVPHY0_LV_FS);
+ val |= u32_encode_bits(settings->nd, LVPHY0_LV_ND);
+ val |= u32_encode_bits(settings->is, LVPHY0_LV_IS);
+
+ regmap_write(tc->regmap, LVPHY0, val | LVPHY0_LV_RST);
+ usleep_range(100, 150);
+ regmap_write(tc->regmap, LVPHY0, val);
+
regmap_write(tc->regmap, SYSRST, SYS_RST_LCD);
- regmap_write(tc->regmap, LVPHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_ND(6));
}
static void tc358775_configure_color_mapping(struct tc_data *tc, u32 fmt)
--
2.39.2
[PATCH 13/20] drm/bridge: tc358775: split the init code
Split the initialization code in tc_bridge_enable() into specific
functions. This is a preparation for further code cleanup and fixes.
No functional change.
While at it, rename tc_bridge_enable() to the more specific
tc358775_bridge_enable().
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 106 --
1 file changed, 66 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index eea41054c6fa..4ec059531c5f 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -308,18 +308,30 @@ static const struct reg_sequence tc_lvmux_jeida18_24[] = {
{ LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R0) },
};
-static void tc_bridge_enable(struct drm_bridge *bridge)
+static void tc358775_configure_dsi(struct tc_data *tc)
+{
+ unsigned int val;
+
+ regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
+ regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
+ regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 3);
+ regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 3);
+
+ val = ((L0EN << tc->num_dsi_lanes) - L0EN) | DSI_CLEN_BIT;
+ regmap_write(tc->regmap, PPI_LANEENABLE, val);
+ regmap_write(tc->regmap, DSI_LANEENABLE, val);
+
+ regmap_write(tc->regmap, PPI_STARTPPI, PPI_START_FUNCTION);
+ regmap_write(tc->regmap, DSI_STARTDSI, DSI_RX_START);
+}
+
+static void tc358775_configure_lvds_timings(struct tc_data *tc,
+ struct drm_display_mode *mode)
{
- struct tc_data *tc = bridge_to_tc(bridge);
u32 hback_porch, hsync_len, hfront_porch, hactive, htime1, htime2;
u32 vback_porch, vsync_len, vfront_porch, vactive, vtime1, vtime2;
- int bpp = mipi_dsi_pixel_format_to_bpp(tc->dsi->format);
- int clkdiv;
- unsigned int val = 0;
- struct drm_display_mode *mode;
- struct drm_connector *connector = get_connector(bridge->encoder);
-
- mode = >encoder->crtc->state->adjusted_mode;
hback_porch = mode->htotal - mode->hsync_end;
hsync_len = mode->hsync_end - mode->hsync_start;
@@ -337,30 +349,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
htime2 = (hfront_porch << 16) + hactive;
vtime2 = (vfront_porch << 16) + vactive;
- regmap_read(tc->regmap, IDREG, );
-
- dev_info(tc->dev, "DSI2LVDS Chip ID.%02x Revision ID. %02x **\n",
-(val >> 8) & 0xFF, val & 0xFF);
-
- regmap_write(tc->regmap, SYSRST,
-SYS_RST_REG | SYS_RST_DSIRX | SYS_RST_BM | SYS_RST_LCD |
-SYS_RST_I2CM);
- usleep_range(3, 4);
-
- regmap_write(tc->regmap, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
- regmap_write(tc->regmap, PPI_LPTXTIMECNT, LPX_PERIOD);
- regmap_write(tc->regmap, PPI_D0S_CLRSIPOCOUNT, 3);
- regmap_write(tc->regmap, PPI_D1S_CLRSIPOCOUNT, 3);
- regmap_write(tc->regmap, PPI_D2S_CLRSIPOCOUNT, 3);
- regmap_write(tc->regmap, PPI_D3S_CLRSIPOCOUNT, 3);
-
- val = ((L0EN << tc->num_dsi_lanes) - L0EN) | DSI_CLEN_BIT;
- regmap_write(tc->regmap, PPI_LANEENABLE, val);
- regmap_write(tc->regmap, DSI_LANEENABLE, val);
-
- regmap_write(tc->regmap, PPI_STARTPPI, PPI_START_FUNCTION);
- regmap_write(tc->regmap, DSI_STARTDSI, DSI_RX_START);
-
/* Video event mode vs pulse mode bit, does not exist for tc358775 */
if (tc->type == TC358765)
val = VPCTRL_EVTMODE;
@@ -381,20 +369,31 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
regmap_write(tc->regmap, VTIM2, vtime2);
regmap_write(tc->regmap, VFUEN, VFUEN_EN);
+}
+
+static void tc358775_configure_pll(struct tc_data *tc, int pixelclk)
+{
regmap_write(tc->regmap, SYSRST, SYS_RST_LCD);
regmap_write(tc->regmap, LVPHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_ND(6));
+}
- dev_dbg(tc->dev, "bus_formats %04x bpc %d\n",
- connector->display_info.bus_formats[0],
- tc->bpc);
- if (connector->display_info.bus_formats[0] ==
MEDIA_BUS_FMT_RGB888_1X7X4_SPWG)
+static void tc358775_configure_color_mapping(struct tc_data *tc, u32 fmt)
+{
+ dev_dbg(tc->dev, "bus_formats %04x bpc %d\n", fmt, tc->bpc);
+
+ if (fmt == MEDIA_BUS_FMT_RGB888_1X7X4_SPWG)
regmap_multi_reg_write(tc->regmap, tc_lvmux_vesa24,
ARRAY_SIZE(tc_lvmux_vesa24));
else
regmap_multi_reg_write(tc->regmap, tc_lvmux_jeida18_24,
[PATCH 12/20] drm/bridge: tc358775: correctly configure LVDS clock
The driver assumes a DSI link with four lanes for now and has the LVDS
clock divider hardcoded to either 3 or 6. Take the number of lanes into
account, too. Also, explicitly set the clock source to the DSI clock.
While at it, replace the TC358775_LVCFG_PCLKDIV() and
TC358775_LVCFG_LVDLINK() inline functions style by the more common
linux bitfields functions.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 48 +--
1 file changed, 21 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index e6d1f0c686ac..eea41054c6fa 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -139,6 +139,12 @@ enum {
};
#define LVCFG 0x049C /* LVDS Configuration */
+#define LVCFG_LVEN BIT(0)
+#define LVCFG_LVDLINK BIT(1)
+#define LVCFG_PCLKDIV GENMASK(7, 4)
+#define LVCFG_PCLKSEL GENMASK(11, 10)
+#define PCLKSEL_HSRCK 0 /* DSI clock */
+
#define LVPHY0 0x04A0 /* LVDS PHY 0 */
#define LV_PHY0_RST(v) FLD_VAL(v, 22, 22) /* PHY reset */
#define LV_PHY0_IS(v) FLD_VAL(v, 15, 14)
@@ -183,28 +189,8 @@ enum {
#define DEBUG01 0x05A4 /* LVDS Data */
#define DSI_CLEN_BIT BIT(0)
-#define DIVIDE_BY_33 /* PCLK=DCLK/3 */
-#define DIVIDE_BY_66 /* PCLK=DCLK/6 */
-#define LVCFG_LVEN_BIT BIT(0)
-
#define L0EN BIT(1)
-#define TC358775_LVCFG_PCLKDIV__MASK 0x00f0
-#define TC358775_LVCFG_PCLKDIV__SHIFT 4
-static inline u32 TC358775_LVCFG_PCLKDIV(uint32_t val)
-{
- return ((val) << TC358775_LVCFG_PCLKDIV__SHIFT) &
- TC358775_LVCFG_PCLKDIV__MASK;
-}
-
-#define TC358775_LVCFG_LVDLINK__MASK 0x0002
-#define TC358775_LVCFG_LVDLINK__SHIFT1
-static inline u32 TC358775_LVCFG_LVDLINK(uint32_t val)
-{
- return ((val) << TC358775_LVCFG_LVDLINK__SHIFT) &
- TC358775_LVCFG_LVDLINK__MASK;
-}
-
enum tc358775_ports {
TC358775_DSI_IN,
TC358775_LVDS_OUT0,
@@ -327,6 +313,8 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
struct tc_data *tc = bridge_to_tc(bridge);
u32 hback_porch, hsync_len, hfront_porch, hactive, htime1, htime2;
u32 vback_porch, vsync_len, vfront_porch, vactive, vtime1, vtime2;
+ int bpp = mipi_dsi_pixel_format_to_bpp(tc->dsi->format);
+ int clkdiv;
unsigned int val = 0;
struct drm_display_mode *mode;
struct drm_connector *connector = get_connector(bridge->encoder);
@@ -408,14 +396,20 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
regmap_write(tc->regmap, VFUEN, VFUEN_EN);
- val = LVCFG_LVEN_BIT;
- if (tc->lvds_dual_link) {
- val |= TC358775_LVCFG_LVDLINK(1);
- val |= TC358775_LVCFG_PCLKDIV(DIVIDE_BY_6);
- } else {
- val |= TC358775_LVCFG_PCLKDIV(DIVIDE_BY_3);
- }
+ /* Configure LVDS clock */
+ clkdiv = bpp / tc->num_dsi_lanes;
+ if (!tc->lvds_dual_link)
+ clkdiv /= 2;
+
+ val = u32_encode_bits(clkdiv, LVCFG_PCLKDIV);
+ val |= u32_encode_bits(PCLKSEL_HSRCK, LVCFG_PCLKSEL);
+ if (tc->lvds_dual_link)
+ val |= LVCFG_LVDLINK;
+
regmap_write(tc->regmap, LVCFG, val);
+
+ /* Finally, enable the LVDS transmitter */
+ regmap_write(tc->regmap, LVCFG, val | LVCFG_LVEN);
}
/*
--
2.39.2
[PATCH 11/20] drm/bridge: tc358775: reformat weird indentation
Reformat the indentation of the mipi_dsi_device_info initialization.
While at it, move it to the top of the function.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index be2175571b99..e6d1f0c686ac 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -557,14 +557,15 @@ static const struct drm_bridge_funcs tc_bridge_funcs = {
static int tc_attach_host(struct tc_data *tc)
{
+ const struct mipi_dsi_device_info info = {
+ .type = "tc358775",
+ .channel = 0,
+ .node = NULL,
+ };
struct device *dev = tc->dev;
struct mipi_dsi_host *host;
struct mipi_dsi_device *dsi;
int ret;
- const struct mipi_dsi_device_info info = { .type = "tc358775",
- .channel = 0,
- .node = NULL,
- };
host = of_find_mipi_dsi_host_by_node(tc->host_node);
if (!host)
--
2.39.2
[PATCH 10/20] drm/bridge: tc358775: simplify lvds_link property
The LVDS link can either be a single link or a dual link. No need for a
u8. Replace it with a bool "lvds_dual_link".
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index a9d731e87970..be2175571b99 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -231,7 +231,7 @@ struct tc_data {
struct regulator*vddio;
struct gpio_desc*reset_gpio;
struct gpio_desc*stby_gpio;
- u8 lvds_link; /* single-link or dual-link */
+ boollvds_dual_link;
u8 bpc;
enum tc3587x5_type type;
@@ -409,7 +409,7 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
regmap_write(tc->regmap, VFUEN, VFUEN_EN);
val = LVCFG_LVEN_BIT;
- if (tc->lvds_link == DUAL_LINK) {
+ if (tc->lvds_dual_link) {
val |= TC358775_LVCFG_LVDLINK(1);
val |= TC358775_LVCFG_PCLKDIV(DIVIDE_BY_6);
} else {
@@ -460,8 +460,8 @@ tc_mode_valid(struct drm_bridge *bridge,
* Maximum pixel clock speed 135MHz for single-link
* 270MHz for dual-link
*/
- if ((mode->clock > 135000 && tc->lvds_link == SINGLE_LINK) ||
- (mode->clock > 27 && tc->lvds_link == DUAL_LINK))
+ if ((mode->clock > 135000 && !tc->lvds_dual_link) ||
+ (mode->clock > 27 && tc->lvds_dual_link))
return MODE_CLOCK_HIGH;
switch (info->bus_formats[0]) {
@@ -516,7 +516,6 @@ static int tc358775_parse_dt(struct device_node *np, struct
tc_data *tc)
of_node_put(tc->host_node);
- tc->lvds_link = SINGLE_LINK;
endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
TC358775_LVDS_OUT1, -1);
if (endpoint) {
@@ -525,13 +524,14 @@ static int tc358775_parse_dt(struct device_node *np,
struct tc_data *tc)
if (remote) {
if (of_device_is_available(remote))
- tc->lvds_link = DUAL_LINK;
+ tc->lvds_dual_link = true;
of_node_put(remote);
}
}
dev_dbg(tc->dev, "no.of dsi lanes: %d\n", tc->num_dsi_lanes);
- dev_dbg(tc->dev, "operating in %d-link mode\n", tc->lvds_link);
+ dev_dbg(tc->dev, "operating in %s-link mode\n",
+ tc->lvds_dual_link ? "dual" : "single");
return 0;
}
--
2.39.2
[PATCH 09/20] drm/bridge: tc358775: remove complex vsdelay calculation
To cite the datasheet on VSDELAY:
During DSI link speed is slower than that of LVDS link’s, data needs
to be buffer within 775XBG before outputting to prevent data from
underflow. Register field VPCTRL[VSDELAY] is used to for this purpose
This driver assumes that the DSI link speed is the pixel clock (as does
every DSI bridge driver), after all the LVDS clock is derived from the
DSI clock. Thus we know for a fact, that the DSI link is not slower than
the LVDS side. Just use the (sane) default value of the bridge and drop
the complicated calculation here.
While at it, replace the TC358775_VPCTRL_MSF() and
TC358775_VPCTRL_OPXLFMT() inline functions by the usual macros for a bit
flag.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 49 +++
1 file changed, 8 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index 54aea58a3406..a9d731e87970 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -109,7 +109,9 @@
#define RDPKTLN 0x0404 /* Command Read Packet Length */
#define VPCTRL 0x0450 /* Video Path Control */
-#define EVTMODEBIT(5) /* Video event mode enable, tc35876x
only */
+#define VPCTRL_MSF BIT(0)
+#define VPCTRL_OPXLFMT BIT(8)
+#define VPCTRL_EVTMODE BIT(5) /* Video event mode enable, tc35876x only */
#define HTIM1 0x0454 /* Horizontal Timing Control 1 */
#define HTIM2 0x0458 /* Horizontal Timing Control 2 */
#define VTIM1 0x045C /* Vertical Timing Control 1 */
@@ -187,30 +189,6 @@ enum {
#define L0EN BIT(1)
-#define TC358775_VPCTRL_VSDELAY__MASK 0x3FF0
-#define TC358775_VPCTRL_VSDELAY__SHIFT 20
-static inline u32 TC358775_VPCTRL_VSDELAY(uint32_t val)
-{
- return ((val) << TC358775_VPCTRL_VSDELAY__SHIFT) &
- TC358775_VPCTRL_VSDELAY__MASK;
-}
-
-#define TC358775_VPCTRL_OPXLFMT__MASK 0x0100
-#define TC358775_VPCTRL_OPXLFMT__SHIFT 8
-static inline u32 TC358775_VPCTRL_OPXLFMT(uint32_t val)
-{
- return ((val) << TC358775_VPCTRL_OPXLFMT__SHIFT) &
- TC358775_VPCTRL_OPXLFMT__MASK;
-}
-
-#define TC358775_VPCTRL_MSF__MASK 0x0001
-#define TC358775_VPCTRL_MSF__SHIFT 0
-static inline u32 TC358775_VPCTRL_MSF(uint32_t val)
-{
- return ((val) << TC358775_VPCTRL_MSF__SHIFT) &
- TC358775_VPCTRL_MSF__MASK;
-}
-
#define TC358775_LVCFG_PCLKDIV__MASK 0x00f0
#define TC358775_LVCFG_PCLKDIV__SHIFT 4
static inline u32 TC358775_LVCFG_PCLKDIV(uint32_t val)
@@ -350,7 +328,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
u32 hback_porch, hsync_len, hfront_porch, hactive, htime1, htime2;
u32 vback_porch, vsync_len, vfront_porch, vactive, vtime1, vtime2;
unsigned int val = 0;
- u16 dsiclk, clkdiv, byteclk, t1, t2, t3, vsdelay;
struct drm_display_mode *mode;
struct drm_connector *connector = get_connector(bridge->encoder);
@@ -398,27 +375,17 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
/* Video event mode vs pulse mode bit, does not exist for tc358775 */
if (tc->type == TC358765)
- val = EVTMODE;
+ val = VPCTRL_EVTMODE;
else
val = 0;
if (tc->bpc == 8)
- val |= TC358775_VPCTRL_OPXLFMT(1);
+ val |= VPCTRL_OPXLFMT;
else /* bpc = 6; */
- val |= TC358775_VPCTRL_MSF(1);
-
- dsiclk = mode->crtc_clock * 3 * tc->bpc / tc->num_dsi_lanes / 1000;
- clkdiv = dsiclk / (tc->lvds_link == DUAL_LINK ? DIVIDE_BY_6 :
DIVIDE_BY_3);
- byteclk = dsiclk / 4;
- t1 = hactive * (tc->bpc * 3 / 8) / tc->num_dsi_lanes;
- t2 = ((10 / clkdiv)) * (hactive + hback_porch + hsync_len +
hfront_porch) / 1000;
- t3 = ((t2 * byteclk) / 100) - (hactive * (tc->bpc * 3 / 8) /
- tc->num_dsi_lanes);
-
- vsdelay = (clkdiv * (t1 + t3) / byteclk) - hback_porch - hsync_len -
hactive;
+ val |= VPCTRL_MSF;
- val |= TC358775_VPCTRL_VSDELAY(vsdelay);
- regmap_write(tc->regmap, VPCTRL, val);
+ regmap_update_bits(tc->regmap, VPCTRL, val,
+ VPCTRL_OPXLFMT | VPCTRL_MSF | VPCTRL_EVTMODE);
regmap_write(tc->regmap, HTIM1, htime1);
regmap_write(tc->regmap, VTIM1, vtime1);
--
2.39.2
[PATCH 08/20] drm/bridge: tc358775: remove error message if regulator is missing
A missing regulator node will automatically be replaced by a dummy. Thus
regulators are optional anyway. Remove the error message.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index b7f15164e655..54aea58a3406 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -675,18 +675,12 @@ static int tc_probe(struct i2c_client *client)
return ret;
tc->vddio = devm_regulator_get(dev, "vddio");
- if (IS_ERR(tc->vddio)) {
- ret = PTR_ERR(tc->vddio);
- dev_err(dev, "vddio-supply not found\n");
- return ret;
- }
+ if (IS_ERR(tc->vddio))
+ return PTR_ERR(tc->vddio);
tc->vdd = devm_regulator_get(dev, "vdd");
- if (IS_ERR(tc->vdd)) {
- ret = PTR_ERR(tc->vdd);
- dev_err(dev, "vdd-supply not found\n");
- return ret;
- }
+ if (IS_ERR(tc->vdd))
+ return PTR_ERR(tc->vdd);
tc->stby_gpio = devm_gpiod_get_optional(dev, "stby", GPIOD_OUT_HIGH);
if (IS_ERR(tc->stby_gpio))
--
2.39.2
[PATCH 07/20] drm/bridge: tc358775: use regmap instead of open coded access functions
The DSI bridge also supports access via DSI in-band reads and writes.
Prepare the driver for that by converting all the access functions to
regmap. This also have the advantage that it will make tracing and
debugging easier and we can use all the bit manipulation helpers from
regmap.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 150 +-
1 file changed, 68 insertions(+), 82 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index 7ae86e8d4c72..b7f15164e655 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
@@ -238,7 +239,7 @@ enum tc3587x5_type {
};
struct tc_data {
- struct i2c_client *i2c;
+ struct regmap *regmap;
struct device *dev;
struct drm_bridge bridge;
@@ -309,42 +310,6 @@ static void tc_bridge_post_disable(struct drm_bridge
*bridge)
usleep_range(1, 11000);
}
-static void d2l_read(struct i2c_client *i2c, u16 addr, u32 *val)
-{
- int ret;
- u8 buf_addr[2];
-
- put_unaligned_be16(addr, buf_addr);
- ret = i2c_master_send(i2c, buf_addr, sizeof(buf_addr));
- if (ret < 0)
- goto fail;
-
- ret = i2c_master_recv(i2c, (u8 *)val, sizeof(*val));
- if (ret < 0)
- goto fail;
-
- pr_debug("d2l: I2C : addr:%04x value:%08x\n", addr, *val);
- return;
-
-fail:
- dev_err(>dev, "Error %d reading from subaddress 0x%x\n",
- ret, addr);
-}
-
-static void d2l_write(struct i2c_client *i2c, u16 addr, u32 val)
-{
- u8 data[6];
- int ret;
-
- put_unaligned_be16(addr, data);
- put_unaligned_le32(val, data + 2);
-
- ret = i2c_master_send(i2c, data, ARRAY_SIZE(data));
- if (ret < 0)
- dev_err(>dev, "Error %d writing to subaddress 0x%x\n",
- ret, addr);
-}
-
/* helper function to access bus_formats */
static struct drm_connector *get_connector(struct drm_encoder *encoder)
{
@@ -358,12 +323,33 @@ static struct drm_connector *get_connector(struct
drm_encoder *encoder)
return NULL;
}
+static const struct reg_sequence tc_lvmux_vesa24[] = {
+ { LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3) },
+ { LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0) },
+ { LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7) },
+ { LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0) },
+ { LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2) },
+ { LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0) },
+ { LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6) },
+};
+
+/* JEIDA-24/JEIDA-18 have the same mapping */
+static const struct reg_sequence tc_lvmux_jeida18_24[] = {
+ { LV_MX0003, LV_MX(LVI_R2, LVI_R3, LVI_R4, LVI_R5) },
+ { LV_MX0407, LV_MX(LVI_R6, LVI_R1, LVI_R7, LVI_G2) },
+ { LV_MX0811, LV_MX(LVI_G3, LVI_G4, LVI_G0, LVI_G1) },
+ { LV_MX1215, LV_MX(LVI_G5, LVI_G6, LVI_G7, LVI_B2) },
+ { LV_MX1619, LV_MX(LVI_B0, LVI_B1, LVI_B3, LVI_B4) },
+ { LV_MX2023, LV_MX(LVI_B5, LVI_B6, LVI_B7, LVI_L0) },
+ { LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R0) },
+};
+
static void tc_bridge_enable(struct drm_bridge *bridge)
{
struct tc_data *tc = bridge_to_tc(bridge);
u32 hback_porch, hsync_len, hfront_porch, hactive, htime1, htime2;
u32 vback_porch, vsync_len, vfront_porch, vactive, vtime1, vtime2;
- u32 val = 0;
+ unsigned int val = 0;
u16 dsiclk, clkdiv, byteclk, t1, t2, t3, vsdelay;
struct drm_display_mode *mode;
struct drm_connector *connector = get_connector(bridge->encoder);
@@ -386,28 +372,29 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
htime2 = (hfront_porch << 16) + hactive;
vtime2 = (vfront_porch << 16) + vactive;
- d2l_read(tc->i2c, IDREG, );
+ regmap_read(tc->regmap, IDREG, );
dev_info(tc->dev, "DSI2LVDS Chip ID.%02x Revision ID. %02x **\n",
(val >> 8) & 0xFF, val & 0xFF);
- d2l_write(tc->i2c, SYSRST, SYS_RST_REG | SYS_RST_DSIRX | SYS_RST_BM |
- SYS_RST_LCD | SYS_RST_I2CM);
+ regmap_write(tc->regmap, SYSRST,
+SYS_RST_REG | SYS_RST_DSIRX | SYS_RST_BM | SYS_RST_LCD |
+SYS_RST_I2CM);
usleep_range(3, 4);
- d2l_write(tc->i2c, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
- d2l_write(tc->i2c, PPI_LPTXTIMECNT, LPX_PERIOD);
- d2l_write(tc->i2c, PPI_D0S_CLRSIPOCOUNT, 3);
- d2l_write(tc->i2c, PPI_D1S_CLRSIPOCOUNT, 3);
- d2l_write(tc->i2c, PPI_D2S_CLRSIPOCOUNT, 3);
- d2l_write(tc->i2c, PPI_D3S_CLRSIPOCOUNT, 3);
+ reg
[PATCH 06/20] drm/bridge: tc358775: redefine LV_MX()
Drop the FLD_VAL macro, just use bit shifts. This is a preparation patch
to switch to regmap and to remove the FLD_VAL().
While at it, reformat the LV_x enum.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 36 ++--
1 file changed, 6 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index 720c0d63fd6a..7ae86e8d4c72 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -124,39 +124,15 @@
#define LV_MX16190x0490 /* Bit 16 to 19 */
#define LV_MX20230x0494 /* Bit 20 to 23 */
#define LV_MX24270x0498 /* Bit 24 to 27 */
-#define LV_MX(b0, b1, b2, b3) (FLD_VAL(b0, 4, 0) | FLD_VAL(b1, 12, 8) | \
- FLD_VAL(b2, 20, 16) | FLD_VAL(b3, 28, 24))
+#define LV_MX(b0, b1, b2, b3) \
+ (((b3) << 24) | ((b2) << 16) | ((b1) << 8) | (b0))
/* Input bit numbers used in mux registers */
enum {
- LVI_R0,
- LVI_R1,
- LVI_R2,
- LVI_R3,
- LVI_R4,
- LVI_R5,
- LVI_R6,
- LVI_R7,
- LVI_G0,
- LVI_G1,
- LVI_G2,
- LVI_G3,
- LVI_G4,
- LVI_G5,
- LVI_G6,
- LVI_G7,
- LVI_B0,
- LVI_B1,
- LVI_B2,
- LVI_B3,
- LVI_B4,
- LVI_B5,
- LVI_B6,
- LVI_B7,
- LVI_HS,
- LVI_VS,
- LVI_DE,
- LVI_L0
+ LVI_R0, LVI_R1, LVI_R2, LVI_R3, LVI_R4, LVI_R5, LVI_R6, LVI_R7,
+ LVI_G0, LVI_G1, LVI_G2, LVI_G3, LVI_G4, LVI_G5, LVI_G6, LVI_G7,
+ LVI_B0, LVI_B1, LVI_B2, LVI_B3, LVI_B4, LVI_B5, LVI_B6, LVI_B7,
+ LVI_HS, LVI_VS, LVI_DE, LVI_L0
};
#define LVCFG 0x049C /* LVDS Configuration */
--
2.39.2
[PATCH 05/20] drm/bridge: tc358775: add crtc modes fixup
The bridge has some limitations regarding the horizontal display
timings. In particular, the pulse width has to be at least 8 pixels
and all horizontal timings have to be a multiple of two pixels, except
for the front porch which is ignored by the bridge anyway.
To accommodate that, add pixels to the pulse width and the back porch
until these requirements are satisfied. The added pixels are then
substracted from the front porch so we don't actually change the pixel
clock (or framerate).
Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 32
1 file changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index 980f71ea5a6a..720c0d63fd6a 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -502,6 +502,37 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
d2l_write(tc->i2c, LVCFG, val);
}
+/*
+ * According to the datasheet, the horizontal back porch, front porch and sync
+ * length must be a multiple of 2 and the minimal horizontal pulse width is 8.
+ * To workaround this, we modify the back porch and the sync pulse width by
+ * adding enough pixels. These pixels will then be substracted from the front
+ * porch which is ignored by the bridge. Hopefully, this marginal modified
+ * timing is tolerated by the panel. The alternative is either a black screen
+ * (if the sync pulse width is too short or a shifted picture if the lengths
+ * are not even).
+ */
+static bool tc_mode_fixup(struct drm_bridge *bridge,
+ const struct drm_display_mode *mode,
+ struct drm_display_mode *adj)
+{
+ u16 hsync_len, hback_porch;
+
+ hback_porch = adj->htotal - adj->hsync_end;
+ if (hback_porch & 1) {
+ adj->hsync_end -= 1;
+ adj->hsync_start -= 1;
+ }
+
+ hsync_len = adj->hsync_end - adj->hsync_start;
+ if (hsync_len < 8)
+ adj->hsync_start -= 8 - hsync_len;
+ else if (hsync_len & 1)
+ adj->hsync_start -= 1;
+
+ return adj->hsync_start >= adj->hdisplay;
+}
+
static enum drm_mode_status
tc_mode_valid(struct drm_bridge *bridge,
const struct drm_display_info *info,
@@ -603,6 +634,7 @@ static const struct drm_bridge_funcs tc_bridge_funcs = {
.attach = tc_bridge_attach,
.pre_enable = tc_bridge_pre_enable,
.enable = tc_bridge_enable,
+ .mode_fixup = tc_mode_fixup,
.mode_valid = tc_mode_valid,
.post_disable = tc_bridge_post_disable,
};
--
2.39.2
[PATCH 04/20] drm/bridge: tc358775: fix regulator supply id
The regulator id is given without the "-supply" postfix. With that
fixed, the driver will look up the correct regulator from the device
tree.
Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index 3b7cc3be2ccd..980f71ea5a6a 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -680,14 +680,14 @@ static int tc_probe(struct i2c_client *client)
if (ret)
return ret;
- tc->vddio = devm_regulator_get(dev, "vddio-supply");
+ tc->vddio = devm_regulator_get(dev, "vddio");
if (IS_ERR(tc->vddio)) {
ret = PTR_ERR(tc->vddio);
dev_err(dev, "vddio-supply not found\n");
return ret;
}
- tc->vdd = devm_regulator_get(dev, "vdd-supply");
+ tc->vdd = devm_regulator_get(dev, "vdd");
if (IS_ERR(tc->vdd)) {
ret = PTR_ERR(tc->vdd);
dev_err(dev, "vdd-supply not found\n");
--
2.39.2
[PATCH 03/20] drm/mediatek: dsi: add support for .dsi_lp11_notity()
drm_bridge_dsi_lp11_notify() shall be called while both the clock and data lanes are still in LP-11 mode. Add the callback. Signed-off-by: Michael Walle --- drivers/gpu/drm/mediatek/mtk_dsi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index ed45c9cc3137..d4a5a2bd591a 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -709,6 +709,7 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi) mtk_dsi_lane_ready(dsi); mtk_dsi_set_mode(dsi); + drm_bridge_dsi_lp11_notify(dsi->next_bridge); mtk_dsi_clk_hs_mode(dsi, 1); dsi->enabled = true; -- 2.39.2
[PATCH 00/20] drm/bridge: tc358775: proper bridge bringup and code cleanup
This patchset fixes the bridge initialization according to the datasheet. Not sure how that even worked before. Maybe because the initialization was done prior to linux (?). The bridge has some peculiarities: (1) The reset has to be deasserted in LP-11 mode (2) For I2C access the bridge needs the DSI clock (3) The bridge has to be configured while the video stream is disabled. (4) The bridge has limitations on the display timings. In particular, the horizontal pulse width has to be at least 8 pixels wide and both the horizontal pulse width as well as the back porch has to be even. According to the datasheet the horizontal front porch as well but in line sync mode, this is ignored. Also line sync is the only supported mode for this bridge, therefore, the front porch is always ignored. The most controversial patch is probably "drm/bridge: add dsi_lp11_notify mechanism" which is needed for (1) above. Some time ago there was a series [1] to add a manual power-up, which was abandoned and which didn't suite the needs for this bridge anyway. Also, this will gradually change the tc_ prefix to tc358775_ while the functions are refactored. The bridge was successfully tested on a Mediatek MT8195 SoC with the following panels: - Innolux G101ICE - AUO G121EAN01.0 - Innolux G156HCE (dual-link LVDS) [1] https://lore.kernel.org/r/20231016165355.1327217-1-dmitry.barysh...@linaro.org/ Signed-off-by: Michael Walle --- Michael Walle (20): drm/bridge: add dsi_lp11_notify mechanism drm/mediatek: dsi: provide LP-11 mode during .pre_enable drm/mediatek: dsi: add support for .dsi_lp11_notity() drm/bridge: tc358775: fix regulator supply id drm/bridge: tc358775: add crtc modes fixup drm/bridge: tc358775: redefine LV_MX() drm/bridge: tc358775: use regmap instead of open coded access functions drm/bridge: tc358775: remove error message if regulator is missing drm/bridge: tc358775: remove complex vsdelay calculation drm/bridge: tc358775: simplify lvds_link property drm/bridge: tc358775: reformat weird indentation drm/bridge: tc358775: correctly configure LVDS clock drm/bridge: tc358775: split the init code drm/bridge: tc358775: configure PLL depending on the LVDS clock drm/bridge: tc358775: dynamically configure DSI link settings drm/bridge: tc358775: use proper defines to configure LVDS timings drm/bridge: tc358775: move bridge power up/down into functions drm/bridge: tc358775: fix the power-up/down delays drm/bridge: tc358775: fix power-up sequencing drm/bridge: tc358775: use devm_drm_bridge_add() drivers/gpu/drm/bridge/Kconfig | 1 + drivers/gpu/drm/bridge/tc358775.c | 601 - drivers/gpu/drm/drm_bridge.c | 16 + drivers/gpu/drm/mediatek/mtk_dsi.c | 8 +- include/drm/drm_bridge.h | 12 + 5 files changed, 355 insertions(+), 283 deletions(-) --- base-commit: 9221b2819b8a4196eecf5476d66201be60fbcf29 change-id: 20240506-tc358775-fix-powerup-53f490043179
[PATCH 02/20] drm/mediatek: dsi: provide LP-11 mode during .pre_enable
As per specification in drivers/gpu/drm/drm_bridge.c the data lanes
should be in LP-11 mode after .pre_enable() has been run. HS mode of the
data lanes are enabled with mtk_dsi_start(). Therefore, move that call
to the .enable() callback.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index c29cc56e..ed45c9cc3137 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -711,8 +711,6 @@ static void mtk_output_dsi_enable(struct mtk_dsi *dsi)
mtk_dsi_set_mode(dsi);
mtk_dsi_clk_hs_mode(dsi, 1);
- mtk_dsi_start(dsi);
-
dsi->enabled = true;
}
@@ -759,7 +757,7 @@ static void mtk_dsi_bridge_atomic_enable(struct drm_bridge
*bridge,
if (dsi->refcount == 0)
return;
- mtk_output_dsi_enable(dsi);
+ mtk_dsi_start(dsi);
}
static void mtk_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
@@ -771,6 +769,9 @@ static void mtk_dsi_bridge_atomic_pre_enable(struct
drm_bridge *bridge,
ret = mtk_dsi_poweron(dsi);
if (ret < 0)
DRM_ERROR("failed to power on dsi\n");
+
+ /* Enter LP-11 state */
+ mtk_output_dsi_enable(dsi);
}
static void mtk_dsi_bridge_atomic_post_disable(struct drm_bridge *bridge,
--
2.39.2
[PATCH 01/20] drm/bridge: add dsi_lp11_notify mechanism
Some bridges have very strict power-up reqirements. In this case, the
Toshiba TC358775. The reset has to be deasserted while *both* the DSI
clock and DSI data lanes are in LP-11 mode. After the reset is relased,
the bridge needs the DSI clock to actually be able to process I2C
access. This access will configure the DSI side of the bridge during
which the DSI data lanes have to be in LP-11 mode. After everything is
configured the video stream can finally be enabled.
This means:
(1) The bridge has to be configured completely in .pre_enable() op
(with the clock turned on and data lanes in LP-11 mode, thus
.pre_enable_prev_first has to be set).
(2) The bridge will enable its output in the .enable() op
(3) There must be some mechanism before (1) where the bridge can
release its reset while the clock lane is still in LP-11 mode.
Unfortunately, (3) is crucial for a correct operation of the bridge.
To satisfy this requriment, introduce a new callback .dsi_lp11_notify()
which will be called by the DSI host driver.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/drm_bridge.c | 16
include/drm/drm_bridge.h | 12
2 files changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 28abe9aa99ca..98cd6558aecb 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1339,6 +1339,22 @@ void drm_bridge_hpd_notify(struct drm_bridge *bridge,
}
EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify);
+/**
+ * drm_bridge_dsi_lp11_notify - notify clock/data lanes LP-11 mode
+ * @bridge: bridge control structure
+ *
+ * DSI host drivers shall call this function while the clock and data lanes
+ * are still in LP-11 mode.
+ *
+ * This function shall be called in a context that can sleep.
+ */
+void drm_bridge_dsi_lp11_notify(struct drm_bridge *bridge)
+{
+ if (bridge->funcs->dsi_lp11_notify)
+ bridge->funcs->dsi_lp11_notify(bridge);
+}
+EXPORT_SYMBOL_GPL(drm_bridge_dsi_lp11_notify);
+
#ifdef CONFIG_OF
/**
* of_drm_find_bridge - find the bridge corresponding to the device node in
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4baca0d9107b..4ef61274e0a8 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -630,6 +630,17 @@ struct drm_bridge_funcs {
*/
void (*hpd_disable)(struct drm_bridge *bridge);
+ /**
+* dsi_lp11_notify:
+*
+* Will be called by the DSI host driver while both the DSI clock
+* lane as well as the DSI data lanes are in LP-11 mode. Some bridges
+* need this state while releasing the reset, for example.
+* Not all DSI host drivers will support this. Therefore, the DSI
+* bridge driver must not rely on this op to be called.
+*/
+ void (*dsi_lp11_notify)(struct drm_bridge *bridge);
+
/**
* @debugfs_init:
*
@@ -898,6 +909,7 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge,
void drm_bridge_hpd_disable(struct drm_bridge *bridge);
void drm_bridge_hpd_notify(struct drm_bridge *bridge,
enum drm_connector_status status);
+void drm_bridge_dsi_lp11_notify(struct drm_bridge *bridge);
#ifdef CONFIG_DRM_PANEL_BRIDGE
bool drm_bridge_is_panel(const struct drm_bridge *bridge);
--
2.39.2
Re: [PATCH v2 0/3] drm/mediatek: Add support for OF graphs
Hi Angelo, On Mon May 6, 2024 at 1:22 PM CEST, AngeloGioacchino Del Regno wrote: > > The problem with this is that you need DDP_COMPONENT_DRM_OVL_ADAPTOR... > > which is > > a software thing and not HW, so that can't be described in devicetree. > > > > The only thing this series won't deal with is exactly that. > > Sorry, no, I got confused. > > The series *does* already deal with that, as the pipeline is built before the > check > for OVL_ADAPTOR components, so that will get probed. Are you sure? Because who is actually adding the OVL_ADAPTOR to the path? It looks like your patch will walk the graph and add all the components according to their compatible string. And since the OVL_ADAPTOR is virtual and doesn't have a node.. -michael
Re: [PATCH v2 0/3] drm/mediatek: Add support for OF graphs
Hi Angelo, On Tue Apr 30, 2024 at 1:33 PM CEST, AngeloGioacchino Del Regno wrote: > >> This series was tested on MT8195 Cherry Tomato and on MT8395 Radxa > >> NIO-12L with both hardcoded paths, OF graph support and partially > >> hardcoded paths (meaning main display through OF graph and external > >> display hardcoded, because of OVL_ADAPTOR). > > > > Is that make sense for you to add the DTS changes of these boards into this > > serie ? > > I asked because, IMHO, that could help to understand the serie. > > > > Yes and no... but I imagine that you're asking this because you're trying to > prepare something with a different SoC+board(s) combination :-) > > In that case, I'm preventively sorry because what follows here is not 100% > perfectly tidy yet as I didn't mean to send the devicetree commits upstream > before this series got picked > > ... but there you go - I'm sure that you won't mind and that the example will > be more than good enough for you. I've tested this series with the DSI0 output and it works. Nice! No need for my DSI0 patch for the MT8395 anymore. But I can't get it to work with the DisplayPort output, that is the dp_intf1/dp_tx interface. I don' know how the pipeline have to look like. The functional spec seems to be ambiguous on this. The text seem to refer to the second vdosys but there is also a diagram where you can use the first vdosys and dsc0. If you have any pointers for me, I'm all ears :) -michael
Re: [PATCH v2 3/3] drm/mediatek: Implement OF graphs support for display paths
Hi Angelo,
On Tue Apr 9, 2024 at 2:02 PM CEST, AngeloGioacchino Del Regno wrote:
> +static int mtk_drm_of_get_ddp_ep_cid(struct device_node *node,
> + int output_port, enum mtk_drm_crtc_path
> crtc_path,
Not sure what's your base branch is, but "enum mtk_drm_crtc_path"
was renamed to "enum mtk_crtc_path" in commit 9e149879038f5
('drm/mediatek: Rename "mtk_drm_crtc" to "mtk_crtc"').
> +/**
> + * mtk_drm_of_ddp_path_build_one - Build a Display HW Pipeline for a CRTC
> Path
> + * @dev: The mediatek-drm device
> + * @cpath:CRTC Path relative to a VDO or MMSYS
> + * @out_path: Pointer to an array that will contain the new pipeline
> + * @out_path_len: Number of entries in the pipeline array
> + *
> + * MediaTek SoCs can use different DDP hardware pipelines (or paths)
> depending
> + * on the board-specific desired display configuration; this function walks
> + * through all of the output endpoints starting from a VDO or MMSYS hardware
> + * instance and builds the right pipeline as specified in device trees.
> + *
> + * Return:
> + * * %0 - Display HW Pipeline successfully built and validated
> + * * %-ENOENT - Display pipeline was not specified in device tree
> + * * %-EINVAL - Display pipeline built but validation failed
> + * * %-ENOMEM - Failure to allocate pipeline array to pass to the caller
> + */
> +static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum
> mtk_drm_crtc_path cpath,
likewise
> + const unsigned int **out_path,
> + unsigned int *out_path_len)
-michael
Re: [PATCH v5 00/10] Improvments for tc358775 with support for tc358765
Hi DRM maintainers, On Sun Feb 25, 2024 at 7:19 AM CET, Tony Lindgren wrote: > Here are v5 patches to improve tc358775 driver and add support for > tc358765. Any news on this series? Is there anything open or can it be merged? FWIW, I have another tc358775 improvement series based on this. -michael signature.asc Description: PGP signature
Re: [PATCH v3 10/10] drm/bridge: tc358775: Configure hs_rate and lp_rate
On Sun Feb 11, 2024 at 10:51 AM CET, Tony Lindgren wrote: > The hs_rate and lp_rate may be used by the dsi host for timing > calculations. The tc358775 has a maximum bit rate of 1 Gbps/lane, > tc358765 has maximurate of 800 Mbps per lane. > > Signed-off-by: Tony Lindgren Reviewed-by: Michael Walle -michael
Re: linux-next: manual merge of the drm-misc tree with Linus' tree
Hi Stephen and all,
Today's linux-next merge of the drm-misc tree got a conflict in:
drivers/gpu/drm/bridge/samsung-dsim.c
between commit:
ff3d5d04db07 ("drm: bridge: samsung-dsim: Don't use
FORCE_STOP_STATE")
from Linus' tree and commit:
b2fe2292624a ("drm: bridge: samsung-dsim: enter display mode in the
enable() callback")
from the drm-misc tree.
I fixed it up (see below, please check) and can carry the fix as
necessary. This is now fixed as far as linux-next is concerned, but
any
non trivial conflicts should be mentioned to your upstream maintainer
when your tree is submitted for merging. You may also want to
consider
cooperating with the maintainer of the conflicting tree to minimise
any
particularly complex conflicts.
I changed my mind and just used the latter version of this file.
Bug wise, this is the wrong solution. Because it will reintroduce the
faulty FORCE_STOP_STATE. Also keep in mind, my fixes commit is/was
already
backported to the stable series.
See also the discussion at [1]. Unfortunately, there was no conculusion
yet.
I think [2] is the proper resolution, at least for the commit
b2fe2292624a
("drm: bridge: samsung-dsim: enter display mode in the enable()
callback")
I'm not sure in what state the drm-misc tree is.
-michael
[1]
https://lore.kernel.org/dri-devel/CAPM=9tytMB9frxNeD08hu1qsusY=wee3bjofmuga1rspabw...@mail.gmail.com/
[2]
https://lore.kernel.org/dri-devel/31e1a38a1d012a32d6f7bc8372b63...@kernel.org/
Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
Hi Dario,
>> Just FYI this conflictted pretty heavily with drm-misc-next changes in
>> the same area, someone should check drm-tip has the correct
>> resolution, I'm not really sure what is definitely should be.
>
> FWIW, this looks rather messy now. The drm-tip doesn't build.
>
> There was a new call to samsung_dsim_set_stop_state() introduced
> in commit b2fe2292624ac (drm: bridge: samsung-dsim: enter display
> mode in the enable() callback).
I had a closer look at the latest linux-next (where somehow my patch
made it into) and tried to apply commit b2fe2292624ac (drm: bridge:
samsung-dsim: enter display mode in the enable() callback). It looks
like only the following hunk is still needed from that patch.
Everything
else is covered by this fixes patch.
Dario, could you rebase your commit onto this patch? I had a quick
test
with this change and it seems to work fine for our case.
--snip--
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 63a1a0c88be4..92755c90e7d2 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1498,6 +1498,8 @@ static void samsung_dsim_atomic_disable(struct
drm_bridge *bridge,
if (!(dsi->state & DSIM_STATE_ENABLED))
return;
+ samsung_dsim_set_display_enable(dsi, false);
+
dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
}
@@ -1506,8 +1508,6 @@ static void
samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
{
struct samsung_dsim *dsi = bridge_to_dsi(bridge);
- samsung_dsim_set_display_enable(dsi, false);
-
dsi->state &= ~DSIM_STATE_ENABLED;
pm_runtime_put_sync(dsi->dev);
}
--snip--
-michael
I'm sorry, but I didn't understand well what I have to do.
Basically, just rebase your patch (drm: bridge: samsung-dsim:
enter display mode in the enable() callback) on top of
linux-next.
This is what I have done:
git clone
https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next.git
cd linux-next
# add your changes, the ones of the emails
git am --reject
0001-drm-bridge-samsung-dsim-enter-display-mode-in-the-en.patch
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 92755c90e7d2..b47929072583 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1508,6 +1508,9 @@ static void
samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
{
struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+ if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
+ samsung_dsim_set_stop_state(dsi, true);
+
This one should be removed. There is no stop state anymore.
With that hunk, it doesn't compile anyway.
dsi->state &= ~DSIM_STATE_ENABLED;
pm_runtime_put_sync(dsi->dev);
}
And then test the driver for my use case.
Yes. The hunk I've posted above, should be all what's left
of your patch, because as far as I see it, most of your changes
are already contained in my fixes patch. What's left is that
you disable the video mode in .disable() and not in
.post_disable().
-michael
Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
Just FYI this conflictted pretty heavily with drm-misc-next changes in
the same area, someone should check drm-tip has the correct
resolution, I'm not really sure what is definitely should be.
FWIW, this looks rather messy now. The drm-tip doesn't build.
There was a new call to samsung_dsim_set_stop_state() introduced
in commit b2fe2292624ac (drm: bridge: samsung-dsim: enter display
mode in the enable() callback).
I had a closer look at the latest linux-next (where somehow my patch
made it into) and tried to apply commit b2fe2292624ac (drm: bridge:
samsung-dsim: enter display mode in the enable() callback). It looks
like only the following hunk is still needed from that patch. Everything
else is covered by this fixes patch.
Dario, could you rebase your commit onto this patch? I had a quick test
with this change and it seems to work fine for our case.
--snip--
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index 63a1a0c88be4..92755c90e7d2 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1498,6 +1498,8 @@ static void samsung_dsim_atomic_disable(struct
drm_bridge *bridge,
if (!(dsi->state & DSIM_STATE_ENABLED))
return;
+ samsung_dsim_set_display_enable(dsi, false);
+
dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
}
@@ -1506,8 +1508,6 @@ static void
samsung_dsim_atomic_post_disable(struct drm_bridge *bridge,
{
struct samsung_dsim *dsi = bridge_to_dsi(bridge);
- samsung_dsim_set_display_enable(dsi, false);
-
dsi->state &= ~DSIM_STATE_ENABLED;
pm_runtime_put_sync(dsi->dev);
}
--snip--
-michael
Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
Also merge commit 663a907e199b (Merge remote-tracking branch 'drm-misc/drm-misc-next' into drm-tip) is broken because it completely removes samsung_dsim_atomic_disable(). Dunno whats going on there. Actually, that merge commit looks even worse. It somehow folds the original samsung_dsim_atomic_disable() into samsung_dsim_atomic_enable(). Therefore, now the enable op will clear the DSIM_STATE_VIDOUT_AVAILABLE flag?! It will also never be set. Dunno how to proceed here. -michael
Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
Just FYI this conflictted pretty heavily with drm-misc-next changes in the same area, someone should check drm-tip has the correct resolution, I'm not really sure what is definitely should be. FWIW, this looks rather messy now. The drm-tip doesn't build. There was a new call to samsung_dsim_set_stop_state() introduced in commit b2fe2292624ac (drm: bridge: samsung-dsim: enter display mode in the enable() callback). Also merge commit 663a907e199b (Merge remote-tracking branch 'drm-misc/drm-misc-next' into drm-tip) is broken because it completely removes samsung_dsim_atomic_disable(). Dunno whats going on there. I'm just about to look at getting it to compile again and I'm trying to retest it. -michael
Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
Hi, Inki, are you picking this up? Or if not, who will? I can pick it up but it would be better to go to the drm-misc tree. If nobody cares about it then I will pick it up. :) acked-by : Inki Dae Who is going to pick this up? Who has access to the drm-misc tree? -michael
Re: [PATCH v2 10/10] drm/bridge: tc358775: Configure hs_rate and lp_rate
> The hs_rate and lp_rate may be used by the dsi host for timing > calculations. The tc358775 has a maximum bit rate of 1 Gbps/lane, > tc358765 has maximurate of 800 Mbps per lane. > > Signed-off-by: Tony Lindgren > --- > drivers/gpu/drm/bridge/tc358775.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/tc358775.c > b/drivers/gpu/drm/bridge/tc358775.c > --- a/drivers/gpu/drm/bridge/tc358775.c > +++ b/drivers/gpu/drm/bridge/tc358775.c > @@ -636,6 +636,11 @@ static int tc_attach_host(struct tc_data *tc) > dsi->format = MIPI_DSI_FMT_RGB888; > dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST | > MIPI_DSI_MODE_LPM; > + if (tc->type == TC358765) > + dsi->hs_rate = 8; It's not clear to me whether this is the data rate or the frequency. From the kernel doc: * @hs_rate: maximum lane frequency for high speed mode in hertz, this should * be set to the real limits of the hardware, zero is only accepted for * legacy drivers The tc358775 datasheet lists 1Gbps per lane, which corresponds to a 500MHz DSI clock frequency. Not sure how that would correspond to the "maximum lane frequency" above. I guess the wording of the comment is just misleading and the value is the data rate of the lane. > + else > + dsi->hs_rate = 10; > + dsi->lp_rate = 1000; That I didn't found in the datasheet. Just a T_min_rx (minimum pulse width response) which is 20ns. But there are no more details on this. -michael
Re: [PATCH v2 06/10] drm/bridge: tc358775: Get bridge data lanes instead of the DSI host lanes
> The current code assumes the data-lanes property is configured on the
> DSI host side instead of the bridge side, and assumes DSI host endpoint 1.
>
> Let's standardize on what the other bridge drivers are doing and parse the
> data-lanes property for the bridge. Only if data-lanes property is not found,
> let's be nice and also check the DSI host for old dtb in use and warn.
>
> Signed-off-by: Tony Lindgren
> ---
> drivers/gpu/drm/bridge/tc358775.c | 25 +++--
> 1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358775.c
> b/drivers/gpu/drm/bridge/tc358775.c
> --- a/drivers/gpu/drm/bridge/tc358775.c
> +++ b/drivers/gpu/drm/bridge/tc358775.c
> @@ -525,27 +525,24 @@ tc_mode_valid(struct drm_bridge *bridge,
> static int tc358775_parse_dt(struct device_node *np, struct tc_data *tc)
> {
> struct device_node *endpoint;
> - struct device_node *parent;
> struct device_node *remote;
> int dsi_lanes = -1;
>
> - /*
> - * To get the data-lanes of dsi, we need to access the dsi0_out of port1
> - * of dsi0 endpoint from bridge port0 of d2l_in
> - */
> endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
>TC358775_DSI_IN, -1);
> - if (endpoint) {
> - /* dsi0_out node */
> - parent = of_graph_get_remote_port_parent(endpoint);
> - of_node_put(endpoint);
> - if (parent) {
> - /* dsi0 port 1 */
> - dsi_lanes = drm_of_get_data_lanes_count_ep(parent, 1,
> -1, 1, 4);
> - of_node_put(parent);
> - }
> + dsi_lanes = drm_of_get_data_lanes_count(endpoint, 1, 4);
> +
> + /* Quirk old dtb: Use data lanes from the DSI host side instead of
> bridge */
> + if (dsi_lanes == -EINVAL || dsi_lanes == -ENODEV) {
> + remote = of_graph_get_remote_endpoint(endpoint);
> + dsi_lanes = drm_of_get_data_lanes_count(remote, 1, 4);
> + of_node_put(remote);
> + if (dsi_lanes >= 1)
> + dev_warn(tc->dev, "missing dsi-lanes property for the
> bridge\n");
It wasn't obvious what this warning should tell me at first. Maybe
add something like ". Falling back to the property of the remote
endpoint". A little verbose, maybe you could come up with a more
dense wording.
In any case,
Reviewed-by: Michael Walle
-michael
Re: [PATCH v2 08/10] drm/bridge: tc358775: Enable pre_enable_prev_first flag
> Set pre_enable_prev_first to ensure the previous bridge is enabled > first. > > Signed-off-by: Tony Lindgren Reviewed-by: Michael Walle Tested-by: Michael Walle
Re: [PATCH v2 07/10] drm/bridge: tc358775: Add burst and low-power modes
> Burst and low-power modes are supported both for tc358765 and tc358775. > > Signed-off-by: Tony Lindgren > --- > drivers/gpu/drm/bridge/tc358775.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/tc358775.c > b/drivers/gpu/drm/bridge/tc358775.c > --- a/drivers/gpu/drm/bridge/tc358775.c > +++ b/drivers/gpu/drm/bridge/tc358775.c > @@ -619,7 +619,8 @@ static int tc_attach_host(struct tc_data *tc) > > dsi->lanes = tc->num_dsi_lanes; > dsi->format = MIPI_DSI_FMT_RGB888; > - dsi->mode_flags = MIPI_DSI_MODE_VIDEO; > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST | > + MIPI_DSI_MODE_LPM; Could you align it with the equal sign of the former line? Reviewed-by: Michael Walle Tested-by: Michael Walle -michael
Re: [PATCH v2 09/10] drm/bridge: tc358775: Add support for tc358765
>> The tc358775 bridge is pin compatible with earlier tc358765 according to
>> the tc358774xbg_datasheet_en_20190118.pdf documentation. Compared to the
>> tc358765, the tc358775 supports a STBY GPIO and higher data rates.
>>
>> The tc358765 has a register bit for video event mode vs video pulse mode.
>> We must set it to video event mode for the LCD output to work, and on the
>> tc358775, this bit no longer exists.
>>
>> Looks like the registers seem to match otherwise based on a quick glance
>> comparing the defines to the earlier Android kernel tc358765 driver.
>>
>> Signed-off-by: Tony Lindgren
>> ---
>> drivers/gpu/drm/bridge/tc358775.c | 26 ++
>> 1 file changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358775.c
>> b/drivers/gpu/drm/bridge/tc358775.c
>> --- a/drivers/gpu/drm/bridge/tc358775.c
>> +++ b/drivers/gpu/drm/bridge/tc358775.c
>> @@ -15,6 +15,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>> #include
>>
>> @@ -107,6 +108,7 @@
>> #define RDPKTLN 0x0404 /* Command Read Packet Length */
>>
>> #define VPCTRL 0x0450 /* Video Path Control */
>> +#define EVTMODEBIT(5) /* Video event mode enable, tc35876x
>> only */
>> #define HTIM1 0x0454 /* Horizontal Timing Control 1 */
>> #define HTIM2 0x0458 /* Horizontal Timing Control 2 */
>> #define VTIM1 0x045C /* Vertical Timing Control 1 */
>> @@ -254,6 +256,11 @@ enum tc358775_ports {
>> TC358775_LVDS_OUT1,
>> };
>>
>> +enum tc3587x5_type {
>> + TC358765,
>
> I'd suggest adding = 1 or =0x65 so that the specified type differs
> from 'no data' = 0 / NULL.
>
>> + TC358775,
>> +};
>> +
>> struct tc_data {
>> struct i2c_client *i2c;
>> struct device *dev;
>> @@ -271,6 +278,8 @@ struct tc_data {
>> struct gpio_desc*stby_gpio;
>> u8 lvds_link; /* single-link or dual-link */
>> u8 bpc;
>> +
>> + enum tc3587x5_type type;
>> };
>>
>> static inline struct tc_data *bridge_to_tc(struct drm_bridge *b)
>> @@ -424,10 +433,16 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
>> d2l_write(tc->i2c, PPI_STARTPPI, PPI_START_FUNCTION);
>> d2l_write(tc->i2c, DSI_STARTDSI, DSI_RX_START);
>>
>> + /* Video event mode vs pulse mode bit, does not exist for tc358775 */
>> + if (tc->type == TC358765)
>> + val = EVTMODE;
>> + else
>> + val = 0;
>> +
>> if (tc->bpc == 8)
>> - val = TC358775_VPCTRL_OPXLFMT(1);
>> + val |= TC358775_VPCTRL_OPXLFMT(1);
>> else /* bpc = 6; */
>> - val = TC358775_VPCTRL_MSF(1);
>> + val |= TC358775_VPCTRL_MSF(1);
>>
>> dsiclk = mode->crtc_clock * 3 * tc->bpc / tc->num_dsi_lanes / 1000;
>> clkdiv = dsiclk / (tc->lvds_link == DUAL_LINK ? DIVIDE_BY_6 :
>> DIVIDE_BY_3);
>> @@ -643,6 +658,7 @@ static int tc_probe(struct i2c_client *client)
>>
>> tc->dev = dev;
>> tc->i2c = client;
>> + tc->type = (enum tc3587x5_type)of_device_get_match_data(dev);
>
> Would it make sense to use i2c_get_match_data() instead?
FWIW, I' planning to add a dsi binding for this driver. So I'd
suggest either the of_ or the device_ variant. Not sure though,
if the new device supports the DSI commands.
Otherwise, the patch looks good:
Reviewed-by: Michael Walle
-michael
>
>>
>> tc->panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node,
>> TC358775_LVDS_OUT0, 0);
>> @@ -704,13 +720,15 @@ static void tc_remove(struct i2c_client *client)
>> }
>>
>> static const struct i2c_device_id tc358775_i2c_ids[] = {
>> - { "tc358775", 0 },
>> + { "tc358765", TC358765, },
>> + { "tc358775", TC358775, },
>> { }
>> };
>> MODULE_DEVICE_TABLE(i2c, tc358775_i2c_ids);
>>
>> static const struct of_device_id tc358775_of_ids[] = {
>> - { .compatible = "toshiba,tc358775", },
>> + { .compatible = "toshiba,tc358765", .data = (void *)TC358765, },
>> + { .compatible = "toshiba,tc358775", .data = (void *)TC358775, },
>> { }
>> };
>> MODULE_DEVICE_TABLE(of, tc358775_of_ids);
>> --
>> 2.43.0
--
With best wishes
Dmitry
Re: [PATCH v2 01/10] dt-bindings: display: bridge: tc358775: make stby gpio and vdd supplies optional
For a normal operation, the vdd supplies nor the stby GPIO is needed. There are boards, where these voltages are statically enabled during board power-up. This means supply is still required. You mean using fixed-regulator? I didn't consider that. But yes, you're right. -michael
Re: [PATCH v2 05/10] drm/bridge: tc358775: make standby GPIO optional
The stby pin is optional. It is only needed for power-up and down sequencing. It is not needed, if the power rails cannot by dynamically enabled. Because the GPIO is not optional, remove the error message. I just noticed a typo: "is *now* optional. -michael
Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
The FORCE_STOP_STATE bit is unsuitable to force the DSI link into LP-11
mode. It seems the bridge internally queues DSI packets and when the
FORCE_STOP_STATE bit is cleared, they are sent in close succession
without any useful timing (this also means that the DSI lanes won't go
into LP-11 mode). The length of this gibberish varies between 1ms and
5ms. This sometimes breaks an attached bridge (TI SN65DSI84 in this
case). In our case, the bridge will fail in about 1 per 500 reboots.
The FORCE_STOP_STATE handling was introduced to have the DSI lanes in
LP-11 state during the .pre_enable phase. But as it turns out, none of
this is needed at all. Between samsung_dsim_init() and
samsung_dsim_set_display_enable() the lanes are already in LP-11 mode.
The code as it was before commit 20c827683de0 ("drm: bridge:
samsung-dsim: Fix init during host transfer") and 0c14d3130654 ("drm:
bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec") was correct
in this regard.
This patch basically reverts both commits. It was tested on an i.MX8M
SoC with an SN65DSI84 bridge. The signals were probed and the DSI
packets were decoded during initialization and link start-up. After
this
patch the first DSI packet on the link is a VSYNC packet and the timing
is correct.
Command mode between .pre_enable and .enable was also briefly tested by
a quick hack. There was no DSI link partner which would have responded,
but it was made sure the DSI packet was send on the link. As a side
note, the command mode seems to just work in HS mode. I couldn't find
that the bridge will handle commands in LP mode.
Fixes: 20c827683de0 ("drm: bridge: samsung-dsim: Fix init during host
transfer")
Fixes: 0c14d3130654 ("drm: bridge: samsung-dsim: Fix i.MX8M enable flow
to meet spec")
Signed-off-by: Michael Walle
---
Let me know wether this should be two commits each reverting one, but
both
commits appeared first in kernel 6.5.
Are there any news?
-michael
Re: [PATCH 4/4] drm/mediatek: support the DSI0 output on the MT8195 VDOSYS0
With the latest dynamic selection of the output component, we can now
support different outputs. Move current output component into the
dynamic routes array and add the new DSI0 output.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 2b0c35cacbc6..6fa88976376e 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -222,7 +222,11 @@ static const unsigned int mt8195_mtk_ddp_main[] =
{
DDP_COMPONENT_DITHER0,
DDP_COMPONENT_DSC0,
DDP_COMPONENT_MERGE0,
- DDP_COMPONENT_DP_INTF0,
Please disregard this patch (the others are ok). There must gone
something
wrong during my testing. DDP_COMPONENT_MERGE0 won't work with
DDP_COMPONENT_DSI0. If anyone has more insights, I'm all ears.
-michael
+};
+
+static const struct mtk_drm_route mt8195_mtk_ddp_main_routes[] = {
+ { 0, DDP_COMPONENT_DP_INTF0 },
+ { 0, DDP_COMPONENT_DSI0 },
};
static const unsigned int mt8195_mtk_ddp_ext[] = {
@@ -308,6 +312,8 @@ static const struct mtk_mmsys_driver_data
mt8192_mmsys_driver_data = {
static const struct mtk_mmsys_driver_data mt8195_vdosys0_driver_data =
{
.main_path = mt8195_mtk_ddp_main,
.main_len = ARRAY_SIZE(mt8195_mtk_ddp_main),
+ .conn_routes = mt8195_mtk_ddp_main_routes,
+ .num_conn_routes = ARRAY_SIZE(mt8195_mtk_ddp_main_routes),
.mmsys_dev_num = 2,
};
Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state
>> >> > DSI device lifetime has three different stages: >> >> > 1. before the DSI link being powered up and clocking, >> >> > 2. when the DSI link is in LP state (for the purpose of this question, >> >> > this is the time between the DSI link being powered up and the video >> >> > stream start) >> >> > 3. when the DSI link is in HS state (while streaming the video). >> >> >> >> It's not clear to me what (2) is. What is the state of the clock and >> >> data lanes? >> > >> > Clk an Data0 should be in the LP mode, ready for LP Data Transfer. >> >> Then this is somehow missing >> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation >> >>A DSI host should keep the PHY powered down until the pre_enable >> operation >>is called. All lanes are in an undefined idle state up to this point, >> and >>it must not be assumed that it is LP-11. pre_enable should initialise >> the >>PHY, set the data lanes to LP-11, and the clock lane to either LP-11 >> or HS >>depending on the mode_flag MIPI_DSI_CLOCK_NON_CONTINUOUS. >> >> So I don't think these three states are sufficient, see below, that >> there >> should be at least four. > >Which one is #4? enabled clock lane (HS mode), data lanes in LP-11 What is the purpose of such a mode? To repeat my first mail: I'm facing similar issues with the tc358775 bridge. This bridge needs to release its reset while both clock and data lanes are in LP-11 mode. But then it needs to be configured (via I2C) while the clock lane is in enabled (HS mode), but the data lanes are still in LP-11 mode. Therefore, for the correct init sequence is: (1) dsi host enables lanes, that is clock and data are in lp-11 (2) dsi bridge driver releases reset of the bridge (3) dsi host enables clock lane, leaves data lanes in lp-11 (4) dsi bridge driver configures the bridge (5) dsi host enables the video stream (6) dsi bridge enables the output port of the bridge -michael >> > I don't think we support ULPS currently. >> > >> > >> >> >> >> I'm facing similar issues with the tc358775 bridge. This bridge needs >> >> to release its reset while both clock and data lanes are in LP-11 >> >> mode. >> >> But then it needs to be configured (via I2C) while the clock lane is >> >> in enabled (HS mode), but the data lanes are still in LP-11 mode. >> >> >> >> To me it looks like there is a fouth case then: >> >> 1. unpowered >> >> 2. DSI clock and data are in LP-11 >> >> 3. DSI clock is in HS and data are in LP-11 >> >> 4. DSI clock is in HS and data is in HS >> >> >> >> (And of course the bridge needs continuous clock mode). >> >> >> >> > Different DSI bridges have different requirements with respect to the >> >> > code being executed at stages 1 and 2. For example several DSI-to-eDP >> >> > bridges (ps8640, tc358767 require for the link to be quiet during >> >> > reset time. >> >> > The DSI-controlled bridges and DSI panels need to send some commands >> >> > in stage 2, before starting up video >> >> > >> >> > In the DRM subsystem stage 3 naturally maps to the >> >> > drm_bridge_funcs::enable, stage 1 also naturally maps to the >> >> > drm_bridge_funcs::pre_enable. Stage 2 doesn't have its own place in >> >> > the DRM call chain. >> >> > Earlier we attempted to solve that using the pre_enable_prev_first, >> >> > which remapped pre-enable callback execution order. However it has led >> >> > us to the two issues. First, at the DSI host driver we do not know >> >> > whether the panel / bridge were updated to use pre_enable_prev_first >> >> > or not. Second, if the bridge has to perform steps during both stages >> >> > 1 and 2, it can not do that. >> >> > >> >> > I'm trying to find a way to express the difference between stages 1 >> >> > and 2 in the generic code, so that we do not to worry about particular >> >> > DSI host and DSI bridge / panel peculiarities when implementing the >> >> > DSI host and/or DSI panel driver. >> >> >> >> For now, I have a rather hacky ".dsi_lp11_notify" callback in >> >> drm_bridge_funcs which is supposed to be called by the DSI host while >> >> the >> >> clock and data lanes are in LP-11 mode. But that is rather an RFC and >> >> me >> >> needing something to get the driver for this bridge working. Because >> >> it's >> >> badly broken. FWIW, you can find my work-in-progress patches at >> >> https://github.com/mwalle/linux/tree/feature-tc358775-fixes >> >> >> >> -michael >> >> >> > >> > >> > -- >> > With best wishes >> > Dmitry > > >
Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state
I'm facing similar issues with the tc358775 bridge. This bridge needs to release its reset while both clock and data lanes are in LP-11 mode. But then it needs to be configured (via I2C) while the clock lane is in enabled (HS mode), but the data lanes are still in LP-11 mode. This is quite an interesting requirement. For example, I'm not 100% sure whether we can get that done on our (msm) hosts. I need to double check that. What frequency is expected on the CLK lane? Can it be an arbitrary frequency or it should be the same freq as the one used later for the video transfer? I presume it has to be the same frequency as the video stream later. That's a least what I have successfully tested. The datasheet doesn't mention if a frequency switch is allowed on the clock lane (which would need a brief switch to LP mode, I presume). I'd say it's not allowed/supported as the bridge is very picky regarding the init sequence in general. I'm using the Mediatek DSI host, where that sequence is possible. I.e. you just enable the clock and data lanes in continuous clock mode, but don't enable the video stream, which should leave the data lanes in LP-11 mode. Sometimes you also have a command mode (instead of a video mode). And if you don't send any commands, the data lanes are in LP-11 mode, too. -michael Therefore, for the correct init sequence is: (1) dsi host enables lanes, that is clock and data are in lp-11 (2) dsi bridge driver releases reset of the bridge (3) dsi host enables clock lane, leaves data lanes in lp-11 (4) dsi bridge driver configures the bridge (5) dsi host enables the video stream (6) dsi bridge enables the output port of the bridge
Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state
[sorry I fat fingered my former reply and converted all CCs to BCCs..] >> >> > DSI device lifetime has three different stages: >> >> > 1. before the DSI link being powered up and clocking, >> >> > 2. when the DSI link is in LP state (for the purpose of this question, >> >> > this is the time between the DSI link being powered up and the video >> >> > stream start) >> >> > 3. when the DSI link is in HS state (while streaming the video). >> >> >> >> It's not clear to me what (2) is. What is the state of the clock and >> >> data lanes? >> > >> > Clk an Data0 should be in the LP mode, ready for LP Data Transfer. >> >> Then this is somehow missing >> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation >> >>A DSI host should keep the PHY powered down until the pre_enable >> operation >>is called. All lanes are in an undefined idle state up to this point, >> and >>it must not be assumed that it is LP-11. pre_enable should initialise >> the >>PHY, set the data lanes to LP-11, and the clock lane to either LP-11 >> or HS >>depending on the mode_flag MIPI_DSI_CLOCK_NON_CONTINUOUS. >> >> So I don't think these three states are sufficient, see below, that >> there >> should be at least four. > >Which one is #4? enabled clock lane (HS mode), data lanes in LP-11 What is the purpose of such a mode? To repeat my first mail: I'm facing similar issues with the tc358775 bridge. This bridge needs to release its reset while both clock and data lanes are in LP-11 mode. But then it needs to be configured (via I2C) while the clock lane is in enabled (HS mode), but the data lanes are still in LP-11 mode. Therefore, for the correct init sequence is: (1) dsi host enables lanes, that is clock and data are in lp-11 (2) dsi bridge driver releases reset of the bridge (3) dsi host enables clock lane, leaves data lanes in lp-11 (4) dsi bridge driver configures the bridge (5) dsi host enables the video stream (6) dsi bridge enables the output port of the bridge -michael >> > I don't think we support ULPS currently. >> > >> > >> >> >> >> I'm facing similar issues with the tc358775 bridge. This bridge needs >> >> to release its reset while both clock and data lanes are in LP-11 >> >> mode. >> >> But then it needs to be configured (via I2C) while the clock lane is >> >> in enabled (HS mode), but the data lanes are still in LP-11 mode. >> >> >> >> To me it looks like there is a fouth case then: >> >> 1. unpowered >> >> 2. DSI clock and data are in LP-11 >> >> 3. DSI clock is in HS and data are in LP-11 >> >> 4. DSI clock is in HS and data is in HS >> >> >> >> (And of course the bridge needs continuous clock mode). >> >> >> >> > Different DSI bridges have different requirements with respect to the >> >> > code being executed at stages 1 and 2. For example several DSI-to-eDP >> >> > bridges (ps8640, tc358767 require for the link to be quiet during >> >> > reset time. >> >> > The DSI-controlled bridges and DSI panels need to send some commands >> >> > in stage 2, before starting up video >> >> > >> >> > In the DRM subsystem stage 3 naturally maps to the >> >> > drm_bridge_funcs::enable, stage 1 also naturally maps to the >> >> > drm_bridge_funcs::pre_enable. Stage 2 doesn't have its own place in >> >> > the DRM call chain. >> >> > Earlier we attempted to solve that using the pre_enable_prev_first, >> >> > which remapped pre-enable callback execution order. However it has led >> >> > us to the two issues. First, at the DSI host driver we do not know >> >> > whether the panel / bridge were updated to use pre_enable_prev_first >> >> > or not. Second, if the bridge has to perform steps during both stages >> >> > 1 and 2, it can not do that. >> >> > >> >> > I'm trying to find a way to express the difference between stages 1 >> >> > and 2 in the generic code, so that we do not to worry about particular >> >> > DSI host and DSI bridge / panel peculiarities when implementing the >> >> > DSI host and/or DSI panel driver. >> >> >> >> For now, I have a rather hacky ".dsi_lp11_notify" callback in >> >> drm_bridge_funcs which is supposed to be called by the DSI host while >> >> the >> >> clock and data lanes are in LP-11 mode. But that is rather an RFC and >> >> me >> >> needing something to get the driver for this bridge working. Because >> >> it's >> >> badly broken. FWIW, you can find my work-in-progress patches at >> >> https://github.com/mwalle/linux/tree/feature-tc358775-fixes >> >> >> >> -michael >> >> >> > >> > >> > -- >> > With best wishes >> > Dmitry > > >
Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state
>> >> > DSI device lifetime has three different stages: >> >> > 1. before the DSI link being powered up and clocking, >> >> > 2. when the DSI link is in LP state (for the purpose of this question, >> >> > this is the time between the DSI link being powered up and the video >> >> > stream start) >> >> > 3. when the DSI link is in HS state (while streaming the video). >> >> >> >> It's not clear to me what (2) is. What is the state of the clock and >> >> data lanes? >> > >> > Clk an Data0 should be in the LP mode, ready for LP Data Transfer. >> >> Then this is somehow missing >> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation >> >>A DSI host should keep the PHY powered down until the pre_enable >> operation >>is called. All lanes are in an undefined idle state up to this point, >> and >>it must not be assumed that it is LP-11. pre_enable should initialise >> the >>PHY, set the data lanes to LP-11, and the clock lane to either LP-11 >> or HS >>depending on the mode_flag MIPI_DSI_CLOCK_NON_CONTINUOUS. >> >> So I don't think these three states are sufficient, see below, that >> there >> should be at least four. > >Which one is #4? enabled clock lane (HS mode), data lanes in LP-11 -michael >> >> > >> > I don't think we support ULPS currently. >> > >> > >> >> >> >> I'm facing similar issues with the tc358775 bridge. This bridge needs >> >> to release its reset while both clock and data lanes are in LP-11 >> >> mode. >> >> But then it needs to be configured (via I2C) while the clock lane is >> >> in enabled (HS mode), but the data lanes are still in LP-11 mode. >> >> >> >> To me it looks like there is a fouth case then: >> >> 1. unpowered >> >> 2. DSI clock and data are in LP-11 >> >> 3. DSI clock is in HS and data are in LP-11 >> >> 4. DSI clock is in HS and data is in HS >> >> >> >> (And of course the bridge needs continuous clock mode). >> >> >> >> > Different DSI bridges have different requirements with respect to the >> >> > code being executed at stages 1 and 2. For example several DSI-to-eDP >> >> > bridges (ps8640, tc358767 require for the link to be quiet during >> >> > reset time. >> >> > The DSI-controlled bridges and DSI panels need to send some commands >> >> > in stage 2, before starting up video >> >> > >> >> > In the DRM subsystem stage 3 naturally maps to the >> >> > drm_bridge_funcs::enable, stage 1 also naturally maps to the >> >> > drm_bridge_funcs::pre_enable. Stage 2 doesn't have its own place in >> >> > the DRM call chain. >> >> > Earlier we attempted to solve that using the pre_enable_prev_first, >> >> > which remapped pre-enable callback execution order. However it has led >> >> > us to the two issues. First, at the DSI host driver we do not know >> >> > whether the panel / bridge were updated to use pre_enable_prev_first >> >> > or not. Second, if the bridge has to perform steps during both stages >> >> > 1 and 2, it can not do that. >> >> > >> >> > I'm trying to find a way to express the difference between stages 1 >> >> > and 2 in the generic code, so that we do not to worry about particular >> >> > DSI host and DSI bridge / panel peculiarities when implementing the >> >> > DSI host and/or DSI panel driver. >> >> >> >> For now, I have a rather hacky ".dsi_lp11_notify" callback in >> >> drm_bridge_funcs which is supposed to be called by the DSI host while >> >> the >> >> clock and data lanes are in LP-11 mode. But that is rather an RFC and >> >> me >> >> needing something to get the driver for this bridge working. Because >> >> it's >> >> badly broken. FWIW, you can find my work-in-progress patches at >> >> https://github.com/mwalle/linux/tree/feature-tc358775-fixes >> >> >> >> -michael >> >> >> > >> > >> > -- >> > With best wishes >> > Dmitry > > >
Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state
> DSI device lifetime has three different stages: > 1. before the DSI link being powered up and clocking, > 2. when the DSI link is in LP state (for the purpose of this question, > this is the time between the DSI link being powered up and the video > stream start) > 3. when the DSI link is in HS state (while streaming the video). It's not clear to me what (2) is. What is the state of the clock and data lanes? Clk an Data0 should be in the LP mode, ready for LP Data Transfer. Then this is somehow missing https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation A DSI host should keep the PHY powered down until the pre_enable operation is called. All lanes are in an undefined idle state up to this point, and it must not be assumed that it is LP-11. pre_enable should initialise the PHY, set the data lanes to LP-11, and the clock lane to either LP-11 or HS depending on the mode_flag MIPI_DSI_CLOCK_NON_CONTINUOUS. So I don't think these three states are sufficient, see below, that there should be at least four. -michael I don't think we support ULPS currently. I'm facing similar issues with the tc358775 bridge. This bridge needs to release its reset while both clock and data lanes are in LP-11 mode. But then it needs to be configured (via I2C) while the clock lane is in enabled (HS mode), but the data lanes are still in LP-11 mode. To me it looks like there is a fouth case then: 1. unpowered 2. DSI clock and data are in LP-11 3. DSI clock is in HS and data are in LP-11 4. DSI clock is in HS and data is in HS (And of course the bridge needs continuous clock mode). > Different DSI bridges have different requirements with respect to the > code being executed at stages 1 and 2. For example several DSI-to-eDP > bridges (ps8640, tc358767 require for the link to be quiet during > reset time. > The DSI-controlled bridges and DSI panels need to send some commands > in stage 2, before starting up video > > In the DRM subsystem stage 3 naturally maps to the > drm_bridge_funcs::enable, stage 1 also naturally maps to the > drm_bridge_funcs::pre_enable. Stage 2 doesn't have its own place in > the DRM call chain. > Earlier we attempted to solve that using the pre_enable_prev_first, > which remapped pre-enable callback execution order. However it has led > us to the two issues. First, at the DSI host driver we do not know > whether the panel / bridge were updated to use pre_enable_prev_first > or not. Second, if the bridge has to perform steps during both stages > 1 and 2, it can not do that. > > I'm trying to find a way to express the difference between stages 1 > and 2 in the generic code, so that we do not to worry about particular > DSI host and DSI bridge / panel peculiarities when implementing the > DSI host and/or DSI panel driver. For now, I have a rather hacky ".dsi_lp11_notify" callback in drm_bridge_funcs which is supposed to be called by the DSI host while the clock and data lanes are in LP-11 mode. But that is rather an RFC and me needing something to get the driver for this bridge working. Because it's badly broken. FWIW, you can find my work-in-progress patches at https://github.com/mwalle/linux/tree/feature-tc358775-fixes -michael -- With best wishes Dmitry
Re: [RFC PATCH 03/10] drm/mipi-dsi: add API for manual control over the DSI link power state
Hi, > DSI device lifetime has three different stages: > 1. before the DSI link being powered up and clocking, > 2. when the DSI link is in LP state (for the purpose of this question, > this is the time between the DSI link being powered up and the video > stream start) > 3. when the DSI link is in HS state (while streaming the video). It's not clear to me what (2) is. What is the state of the clock and data lanes? I'm facing similar issues with the tc358775 bridge. This bridge needs to release its reset while both clock and data lanes are in LP-11 mode. But then it needs to be configured (via I2C) while the clock lane is in enabled (HS mode), but the data lanes are still in LP-11 mode. To me it looks like there is a fouth case then: 1. unpowered 2. DSI clock and data are in LP-11 3. DSI clock is in HS and data are in LP-11 4. DSI clock is in HS and data is in HS (And of course the bridge needs continuous clock mode). > Different DSI bridges have different requirements with respect to the > code being executed at stages 1 and 2. For example several DSI-to-eDP > bridges (ps8640, tc358767 require for the link to be quiet during > reset time. > The DSI-controlled bridges and DSI panels need to send some commands > in stage 2, before starting up video > > In the DRM subsystem stage 3 naturally maps to the > drm_bridge_funcs::enable, stage 1 also naturally maps to the > drm_bridge_funcs::pre_enable. Stage 2 doesn't have its own place in > the DRM call chain. > Earlier we attempted to solve that using the pre_enable_prev_first, > which remapped pre-enable callback execution order. However it has led > us to the two issues. First, at the DSI host driver we do not know > whether the panel / bridge were updated to use pre_enable_prev_first > or not. Second, if the bridge has to perform steps during both stages > 1 and 2, it can not do that. > > I'm trying to find a way to express the difference between stages 1 > and 2 in the generic code, so that we do not to worry about particular > DSI host and DSI bridge / panel peculiarities when implementing the > DSI host and/or DSI panel driver. For now, I have a rather hacky ".dsi_lp11_notify" callback in drm_bridge_funcs which is supposed to be called by the DSI host while the clock and data lanes are in LP-11 mode. But that is rather an RFC and me needing something to get the driver for this bridge working. Because it's badly broken. FWIW, you can find my work-in-progress patches at https://github.com/mwalle/linux/tree/feature-tc358775-fixes -michael
Re: [PATCH 2/6] drm/bridge: tc358775: Fix getting dsi host data lanes
+ dt maintainers I actually have the same fix, but with one additional detail, which I'm unsure about though: This looks at the data-lanes property of the *remote* endpoint whereas other bridge drivers (see tc358767, ti-sn65dsi83, lt8912b, anx7625) look at the local endpoint and I'm not sure what is correct. Yes I've been wondering about that too. Let's just move it over to the bridge node? We could produce a warning if the dsi host node has the data-lanes property.. No current in kernel users AFAIK. I haven't found any in-tree users either. In my patch, I first try the remote end and then the local end. But thinking more about it I don't think this is correct. Maybe we can do it the other way around, first try data-lanes of the local endpoint and if not found, then try the remote one. That way, we would at least be backwards compatible in the driver. And for the dt-bindings, make it mandatory to have a local data-lanes. -michael FYI, for omapdrm, we already have a legacy dt property "lanes" for the wiring that tells number of lanes used and the order of the lanes. Regards, Tony
Re: [PATCH 3/6] drm/bridge: tc358775: Add jeida-24 support
> The jeida-24 register values are the default hardware settings, but they
> not listed in the driver. Let's add them.
jeida-24 and jeida-18 should have the same mapping, jeida-18 is broken in
this driver. could you test this patch:
--snip--
>From 46da1d76d4908e5879ed746cce1faeacd69c432e Mon Sep 17 00:00:00 2001
From: Michael Walle
Date: Wed, 4 Oct 2023 13:52:57 +0200
Subject: [PATCH] drm/bridge: tc358775: fix support for jeida-18 and jeida-24
The bridge always uses 24bpp internally. Therefore, for jeida-18
mapping we need to discard the lowest two bits for each channel and thus
starting with LV_[RGB]2. jeida-24 has the same mapping but uses four
lanes instead of three, with the forth pair transmitting the lowest two
bits of each channel. Thus, the mapping between jeida-18 and jeida-24
is actually the same, except that one channel is turned off (by
selecting the RGB666 format in VPCTRL).
While at it, remove the bogus comment about the hardware default because
the default is overwritten in any case.
Tested with a jeida-18 display (Evervision VGG644804).
Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
Signed-off-by: Michael Walle
---
drivers/gpu/drm/bridge/tc358775.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358775.c
b/drivers/gpu/drm/bridge/tc358775.c
index 90a89d70d832..592c69c2aedc 100644
--- a/drivers/gpu/drm/bridge/tc358775.c
+++ b/drivers/gpu/drm/bridge/tc358775.c
@@ -454,10 +454,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
dev_dbg(tc->dev, "bus_formats %04x bpc %d\n",
connector->display_info.bus_formats[0],
tc->bpc);
- /*
-* Default hardware register settings of tc358775 configured
-* with MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA jeida-24 format
-*/
if (connector->display_info.bus_formats[0] ==
MEDIA_BUS_FMT_RGB888_1X7X4_SPWG) {
/* VESA-24 */
@@ -468,14 +464,15 @@ static void tc_bridge_enable(struct drm_bridge *bridge)
d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1,
LVI_B2));
d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5,
LVI_L0));
d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE,
LVI_R6));
- } else { /* MEDIA_BUS_FMT_RGB666_1X7X3_SPWG - JEIDA-18 */
- d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2,
LVI_R3));
- d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R4, LVI_L0, LVI_R5,
LVI_G0));
- d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_L0,
LVI_L0));
- d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5,
LVI_B0));
- d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_L0, LVI_L0, LVI_B1,
LVI_B2));
- d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5,
LVI_L0));
- d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE,
LVI_L0));
+ } else {
+ /* JEIDA-18 and JEIDA-24 */
+ d2l_write(tc->i2c, LV_MX0003, LV_MX(LVI_R2, LVI_R3, LVI_R4,
LVI_R5));
+ d2l_write(tc->i2c, LV_MX0407, LV_MX(LVI_R6, LVI_R1, LVI_R7,
LVI_G2));
+ d2l_write(tc->i2c, LV_MX0811, LV_MX(LVI_G3, LVI_G4, LVI_G0,
LVI_G1));
+ d2l_write(tc->i2c, LV_MX1215, LV_MX(LVI_G5, LVI_G6, LVI_G7,
LVI_B2));
+ d2l_write(tc->i2c, LV_MX1619, LV_MX(LVI_B0, LVI_B1, LVI_B3,
LVI_B4));
+ d2l_write(tc->i2c, LV_MX2023, LV_MX(LVI_B5, LVI_B6, LVI_B7,
LVI_L0));
+ d2l_write(tc->i2c, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE,
LVI_R0));
}
d2l_write(tc->i2c, VFUEN, VFUEN_EN);
--
2.39.2
Re: [PATCH 2/6] drm/bridge: tc358775: Fix getting dsi host data lanes
> The current code assume hardcoded dsi host endpoint 1, which may not
> be the case. Let's fix that and simplify the code by getting the dsi
> endpoint with of_graph_get_remote_endpoint() that does not assume any
> endpoint numbering.
>
> Fixes: b26975593b17 ("display/drm/bridge: TC358775 DSI/LVDS driver")
> Signed-off-by: Tony Lindgren
> ---
> drivers/gpu/drm/bridge/tc358775.c | 16
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358775.c
> b/drivers/gpu/drm/bridge/tc358775.c
> --- a/drivers/gpu/drm/bridge/tc358775.c
> +++ b/drivers/gpu/drm/bridge/tc358775.c
> @@ -528,25 +528,17 @@ tc_mode_valid(struct drm_bridge *bridge,
> static int tc358775_parse_dt(struct device_node *np, struct tc_data *tc)
> {
> struct device_node *endpoint;
> - struct device_node *parent;
> struct device_node *remote;
> int dsi_lanes = -1;
>
> - /*
> - * To get the data-lanes of dsi, we need to access the dsi0_out of port1
> - * of dsi0 endpoint from bridge port0 of d2l_in
> - */
> endpoint = of_graph_get_endpoint_by_regs(tc->dev->of_node,
>TC358775_DSI_IN, -1);
> if (endpoint) {
> - /* dsi0_out node */
> - parent = of_graph_get_remote_port_parent(endpoint);
> + /* Get the configured data lanes on the dsi host side */
> + remote = of_graph_get_remote_endpoint(endpoint);
> of_node_put(endpoint);
> - if (parent) {
> - /* dsi0 port 1 */
> - dsi_lanes = drm_of_get_data_lanes_count_ep(parent, 1,
> -1, 1, 4);
> - of_node_put(parent);
> - }
> + dsi_lanes = drm_of_get_data_lanes_count(remote, 1, 4);
I actually have the same fix, but with one additional detail, which I'm
unsure about though: This looks at the data-lanes property of the *remote*
endpoint whereas other bridge drivers (see tc358767, ti-sn65dsi83, lt8912b,
anx7625) look at the local endpoint and I'm not sure what is correct.
-michael
Re: [PATCH 1/6] dt-bindings: tc358775: Add support for tc358765
Hi, > The tc358765 is similar to tc358775 except for the stdby-gpios. Bad timing (for me). I'm about to send a bigger patch series for the tc358775 which fixes the (completely) broken initialialization. And also contains some of your fixes. That being said, I intend to make the standby gpio optional also for the tc358755, because it might just be hardwired on the board. But second, I'm really curious if this bridge is working for you correctly as it is at the moment. One particular thing I've noticed is that you must release the reset while both the clock and the data lanes are in LP11 mode. Otherwise, the bridge won't work properly (i.e. horizontally shifted picture, or no picture at all). What DSI host controller are you using? -michael
Re: [PATCH v4 2/2] drm/mediatek: dpi/dsi: fix possible_crtcs calculation
Hi,
> mtk_drm_find_possible_crtc_by_comp() assumed that the main path will
> always have the CRTC with id 0, the ext id 1 and the third id 2. This
> is only true if the paths are all available. But paths are optional
> (see
> also comment in mtk_drm_kms_init()), e.g. the main path might not be
> enabled or available at all. Then the CRTC IDs will shift one up, e.g.
> ext will be 0 and the third path will be 1.
>
> To fix that, dynamically calculate the IDs by the presence of the
> paths.
>
> While at it, make the return code a signed one and return -ENOENT if no
> path is found and handle the error in the callers.
>
> Fixes: 5aa8e7647676 ("drm/mediatek: dpi/dsi: Change the getting
> possible_crtc way")
> Suggested-by: Nícolas F. R. A. Prado
> Signed-off-by: Michael Walle
> Reviewed-by: Nícolas F. R. A. Prado
> Tested-by: Nícolas F. R. A. Prado
Is there anything wrong with these two patches? They are now lingering
around for more than two months.
Unfortunately, patch 2/2 won't apply anymore because of commit
01389b324c97 ("drm/mediatek: Add connector dynamic selection
capability). And I'm a bit puzzled for what the crtc_id is used
there because I guess it will have the same problem this patch
fixes.
Please base on the latest kernel version to fix.
Of course, but the question is how to fix it. Maybe Jason-JH.Lin
can help here.
In any case, please pick patch 1/2, it's independent of the second
patch and should still apply.
-michael
[PATCH] dt-bindings: display: mediatek: dsi: remove Xinlei's mail
Xinlei Lee's mail is bouncing: : host mailgw02.mediatek.com[216.200.240.185] said: 550 Relaying mail to xinlei@mediatek.com is not allowed (in reply to RCPT TO command) Remove it. Signed-off-by: Michael Walle --- .../devicetree/bindings/display/mediatek/mediatek,dsi.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml index 4a7a9ff21996..8611319bed2e 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml @@ -10,7 +10,6 @@ maintainers: - Chun-Kuang Hu - Philipp Zabel - Jitao Shi - - Xinlei Lee description: | The MediaTek DSI function block is a sink of the display subsystem and can -- 2.39.2
[PATCH 4/4] drm/mediatek: support the DSI0 output on the MT8195 VDOSYS0
With the latest dynamic selection of the output component, we can now
support different outputs. Move current output component into the
dynamic routes array and add the new DSI0 output.
Signed-off-by: Michael Walle
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 2b0c35cacbc6..6fa88976376e 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -222,7 +222,11 @@ static const unsigned int mt8195_mtk_ddp_main[] = {
DDP_COMPONENT_DITHER0,
DDP_COMPONENT_DSC0,
DDP_COMPONENT_MERGE0,
- DDP_COMPONENT_DP_INTF0,
+};
+
+static const struct mtk_drm_route mt8195_mtk_ddp_main_routes[] = {
+ { 0, DDP_COMPONENT_DP_INTF0 },
+ { 0, DDP_COMPONENT_DSI0 },
};
static const unsigned int mt8195_mtk_ddp_ext[] = {
@@ -308,6 +312,8 @@ static const struct mtk_mmsys_driver_data
mt8192_mmsys_driver_data = {
static const struct mtk_mmsys_driver_data mt8195_vdosys0_driver_data = {
.main_path = mt8195_mtk_ddp_main,
.main_len = ARRAY_SIZE(mt8195_mtk_ddp_main),
+ .conn_routes = mt8195_mtk_ddp_main_routes,
+ .num_conn_routes = ARRAY_SIZE(mt8195_mtk_ddp_main_routes),
.mmsys_dev_num = 2,
};
--
2.39.2
[PATCH 3/4] arm64: dts: mediatek: mt8195: add DSI and MIPI DPHY nodes
Add the two DSI controller node and the associated DPHY nodes.
Individual boards have to enable them in the board device tree.
Signed-off-by: Michael Walle
---
arch/arm64/boot/dts/mediatek/mt8195.dtsi | 48
1 file changed, 48 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 54c674c45b49..0621bb461967 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -1714,6 +1714,26 @@ u2port3: usb-phy@0 {
};
};
+ mipi_tx0: dsi-phy@11c8 {
+ compatible = "mediatek,mt8195-mipi-tx",
"mediatek,mt8183-mipi-tx";
+ reg = <0 0x11c8 0 0x1000>;
+ clocks = <>;
+ clock-output-names = "mipi_tx0_pll";
+ #clock-cells = <0>;
+ #phy-cells = <0>;
+ status = "disabled";
+ };
+
+ mipi_tx1: dsi-phy@11c9 {
+ compatible = "mediatek,mt8195-mipi-tx",
"mediatek,mt8183-mipi-tx";
+ reg = <0 0x11c9 0 0x1000>;
+ clocks = <>;
+ clock-output-names = "mipi_tx1_pll";
+ #clock-cells = <0>;
+ #phy-cells = <0>;
+ status = "disabled";
+ };
+
i2c5: i2c@11d0 {
compatible = "mediatek,mt8195-i2c",
"mediatek,mt8192-i2c";
@@ -2737,6 +2757,20 @@ dither0: dither@1c007000 {
mediatek,gce-client-reg = < SUBSYS_1c00 0x7000
0x1000>;
};
+ dsi0: dsi@1c008000 {
+ compatible = "mediatek,mt8195-dsi",
"mediatek,mt8183-dsi";
+ reg = <0 0x1c008000 0 0x1000>;
+ interrupts = ;
+ power-domains = < MT8195_POWER_DOMAIN_VDOSYS0>;
+ clocks = < CLK_VDO0_DSI0>,
+< CLK_VDO0_DSI0_DSI>,
+<_tx0>;
+ clock-names = "engine", "digital", "hs";
+ phys = <_tx0>;
+ phy-names = "dphy";
+ status = "disabled";
+ };
+
dsc0: dsc@1c009000 {
compatible = "mediatek,mt8195-disp-dsc";
reg = <0 0x1c009000 0 0x1000>;
@@ -2746,6 +2780,20 @@ dsc0: dsc@1c009000 {
mediatek,gce-client-reg = < SUBSYS_1c00 0x9000
0x1000>;
};
+ dsi1: dsi@1c012000 {
+ compatible = "mediatek,mt8195-dsi",
"mediatek,mt8183-dsi";
+ reg = <0 0x1c012000 0 0x1000>;
+ interrupts = ;
+ power-domains = < MT8195_POWER_DOMAIN_VDOSYS0>;
+ clocks = < CLK_VDO0_DSI1>,
+< CLK_VDO0_DSI1_DSI>,
+<_tx1>;
+ clock-names = "engine", "digital", "hs";
+ phys = <_tx1>;
+ phy-names = "dphy";
+ status = "disabled";
+ };
+
merge0: merge@1c014000 {
compatible = "mediatek,mt8195-disp-merge";
reg = <0 0x1c014000 0 0x1000>;
--
2.39.2
[PATCH 1/4] dt-bindings: display: mediatek: dsi: add compatible for MediaTek MT8195
Add the compatible string for MediaTek MT8195 SoC, using the same DSI block as the MT8183. Signed-off-by: Michael Walle --- .../devicetree/bindings/display/mediatek/mediatek,dsi.yaml| 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml index 537e5304b730..4a7a9ff21996 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml @@ -35,6 +35,10 @@ properties: - enum: - mediatek,mt6795-dsi - const: mediatek,mt8173-dsi + - items: + - enum: + - mediatek,mt8195-dsi + - const: mediatek,mt8183-dsi reg: maxItems: 1 -- 2.39.2
[PATCH 2/4] dt-bindings: phy: add compatible for Mediatek MT8195
Add the compatible string for MediaTek MT8195 SoC, using the same MIPI D-PHY block as the MT8183. Signed-off-by: Michael Walle --- Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml index 6703689fcdbe..f6e494d0d89b 100644 --- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml +++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml @@ -31,6 +31,7 @@ properties: - items: - enum: - mediatek,mt8188-mipi-tx + - mediatek,mt8195-mipi-tx - mediatek,mt8365-mipi-tx - const: mediatek,mt8183-mipi-tx - const: mediatek,mt2701-mipi-tx -- 2.39.2
[PATCH 0/4] drm/mediatek: support DSI output on MT8195
Add support for a DSI output on VDOSYS0. Luckily, there is a new feature to support dynamic selections of the output (connector). Use it to add support for a DSI output. Apart from that, this is pretty straghtforward by just adding new compatibles and add the correct DSI nodes to the SoC dtsi. This was tested with a Toshiba TC358775 DSI-to-LVDS bridge and three different panels. Please note, that the driver for this bridge doesn't work well and needs a more or less complete rework, which will be posted on a separate series. Michael Walle (4): dt-bindings: display: mediatek: dsi: add compatible for MediaTek MT8195 dt-bindings: phy: add compatible for Mediatek MT8195 arm64: dts: mediatek: mt8195: add DSI and MIPI DPHY nodes drm/mediatek: support the DSI0 output on the MT8195 VDOSYS0 .../display/mediatek/mediatek,dsi.yaml| 4 ++ .../bindings/phy/mediatek,dsi-phy.yaml| 1 + arch/arm64/boot/dts/mediatek/mt8195.dtsi | 48 +++ drivers/gpu/drm/mediatek/mtk_drm_drv.c| 8 +++- 4 files changed, 60 insertions(+), 1 deletion(-) -- 2.39.2
[PATCH] phy: mediatek: mipi: mt8183: fix minimal supported frequency
The lowest supported clock frequency of the PHY is 125MHz (see also
mtk_mipi_tx_pll_enable()), but the clamping in .round_rate() has the
wrong minimal value, which will make the .enable() op return -EINVAL on
low frequencies. Fix the minimal clamping value.
Fixes: efda51a58b4a ("drm/mediatek: add mipi_tx driver for mt8183")
Signed-off-by: Michael Walle
---
drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8183.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8183.c
b/drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8183.c
index f021ec5a70e5..553725e1269c 100644
--- a/drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8183.c
+++ b/drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8183.c
@@ -100,7 +100,7 @@ static void mtk_mipi_tx_pll_disable(struct clk_hw *hw)
static long mtk_mipi_tx_pll_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long *prate)
{
- return clamp_val(rate, 5000, 16);
+ return clamp_val(rate, 12500, 16);
}
static const struct clk_ops mtk_mipi_tx_pll_ops = {
--
2.39.2
[PATCH 2/2] drm/panel-simple: add Evervision VGG644804 panel entry
Timings taken from the datasheet, although sometimes there are just
typical values and it's not clear if they are no min and max values or
if you must use the typical value exactly. To make things worse, there
is no back porch but only a combined sync and back porch length.
Unfortunately, there is not public datasheet. Therefore, here are the
relevant timings:
| min | typ | max |
-+-++-+
CLK frequency| - | 25.175 | - |
HS period| - | 800 | - |
HS pulse width | 5 |30 | - |
HS-DEN time | 112 | 144 | 175 |
DEN pulse width | - | 640 | - |
VS pulse width | 1 | 3 | 5 |
VS-DEN time | - |35 | - |
VS period| - | 525 | - |
Signed-off-by: Michael Walle
---
drivers/gpu/drm/panel/panel-simple.c | 30
1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 9367a4572dcf..26702a847b63 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1973,6 +1973,33 @@ static const struct panel_desc eink_vb3300_kca = {
.connector_type = DRM_MODE_CONNECTOR_DPI,
};
+static const struct display_timing evervision_vgg644804_timing = {
+ .pixelclock = { 25175000, 25175000, 25175000 },
+ .hactive = { 640, 640, 640 },
+ .hfront_porch = { 16, 16, 16 },
+ .hback_porch = { 82, 114, 170 },
+ .hsync_len = { 5, 30, 30 },
+ .vactive = { 480, 480, 480 },
+ .vfront_porch = { 10, 10, 10 },
+ .vback_porch = { 30, 32, 34 },
+ .vsync_len = { 1, 3, 5 },
+ .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
+DISPLAY_FLAGS_DE_HIGH | DISPLAY_FLAGS_PIXDATA_POSEDGE |
+DISPLAY_FLAGS_SYNC_POSEDGE,
+};
+
+static const struct panel_desc evervision_vgg644804 = {
+ .timings = _vgg644804_timing,
+ .num_timings = 1,
+ .bpc = 8,
+ .size = {
+ .width = 115,
+ .height = 86,
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB666_1X7X3_SPWG,
+ .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE,
+};
+
static const struct display_timing evervision_vgg804821_timing = {
.pixelclock = { 2760, 3330, 5000 },
.hactive = { 800, 800, 800 },
@@ -4334,6 +4361,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "eink,vb3300-kca",
.data = _vb3300_kca,
+ }, {
+ .compatible = "evervision,vgg644804",
+ .data = _vgg644804,
}, {
.compatible = "evervision,vgg804821",
.data = _vgg804821,
--
2.39.2
[PATCH 1/2] dt-bindings: display: simple: add Evervision VGG644804 panel
Add Evervision VGG644804 5.7" 640x480 LVDS panel compatible string. Signed-off-by: Michael Walle --- .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index 3ec9ee95045f..2471c99a0c96 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -144,6 +144,8 @@ properties: - edt,etmv570g2dhu # E Ink VB3300-KCA - eink,vb3300-kca +# Evervision Electronics Co. Ltd. VGG644804 5.7" VGA TFT LCD Panel + - evervision,vgg644804 # Evervision Electronics Co. Ltd. VGG804821 5.0" WVGA TFT LCD Panel - evervision,vgg804821 # Foxlink Group 5" WVGA TFT LCD panel -- 2.39.2
Re: [PATCH v4 2/2] drm/mediatek: dpi/dsi: fix possible_crtcs calculation
Hi,
mtk_drm_find_possible_crtc_by_comp() assumed that the main path will
always have the CRTC with id 0, the ext id 1 and the third id 2. This
is only true if the paths are all available. But paths are optional
(see
also comment in mtk_drm_kms_init()), e.g. the main path might not be
enabled or available at all. Then the CRTC IDs will shift one up, e.g.
ext will be 0 and the third path will be 1.
To fix that, dynamically calculate the IDs by the presence of the
paths.
While at it, make the return code a signed one and return -ENOENT if no
path is found and handle the error in the callers.
Fixes: 5aa8e7647676 ("drm/mediatek: dpi/dsi: Change the getting
possible_crtc way")
Suggested-by: Nícolas F. R. A. Prado
Signed-off-by: Michael Walle
Reviewed-by: Nícolas F. R. A. Prado
Tested-by: Nícolas F. R. A. Prado
Is there anything wrong with these two patches? They are now lingering
around for more than two months.
Unfortunately, patch 2/2 won't apply anymore because of commit
01389b324c97 ("drm/mediatek: Add connector dynamic selection
capability). And I'm a bit puzzled for what the crtc_id is used
there because I guess it will have the same problem this patch
fixes.
-michael
---
v4:
- return -ENOENT if mtk_drm_find_possible_crtc_by_comp() doesn't find
any path
v3:
- use data instead of priv_n->data
- fixed typos
- collected Rb and Tb tags
v2:
- iterate over all_drm_private[] to get any vdosys
- new check if a path is available
---
drivers/gpu/drm/mediatek/mtk_dpi.c | 5 +-
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 75 -
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 3 +-
drivers/gpu/drm/mediatek/mtk_dsi.c | 5 +-
4 files changed, 68 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 2f931e4e2b60..f9250f7ee706 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -796,7 +796,10 @@ static int mtk_dpi_bind(struct device *dev, struct
device *master, void *data)
return ret;
}
- dpi->encoder.possible_crtcs =
mtk_drm_find_possible_crtc_by_comp(drm_dev, dpi->dev);
+ ret = mtk_drm_find_possible_crtc_by_comp(drm_dev, dpi->dev);
+ if (ret < 0)
+ goto err_cleanup;
+ dpi->encoder.possible_crtcs = ret;
ret = drm_bridge_attach(>encoder, >bridge, NULL,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 771f4e173353..83ae75ecd858 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -507,6 +507,27 @@ static bool mtk_drm_find_comp_in_ddp(struct device
*dev,
return false;
}
+static bool mtk_ddp_path_available(const unsigned int *path,
+ unsigned int path_len,
+ struct device_node **comp_node)
+{
+ unsigned int i;
+
+ if (!path)
+ return false;
+
+ for (i = 0U; i < path_len; i++) {
+ /* OVL_ADAPTOR doesn't have a device node */
+ if (path[i] == DDP_COMPONENT_DRM_OVL_ADAPTOR)
+ continue;
+
+ if (!comp_node[path[i]])
+ return false;
+ }
+
+ return true;
+}
+
int mtk_ddp_comp_get_id(struct device_node *node,
enum mtk_ddp_comp_type comp_type)
{
@@ -522,25 +543,47 @@ int mtk_ddp_comp_get_id(struct device_node *node,
return -EINVAL;
}
-unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device
*drm,
- struct device *dev)
+int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm, struct
device *dev)
{
struct mtk_drm_private *private = drm->dev_private;
- unsigned int ret = 0;
-
- if (mtk_drm_find_comp_in_ddp(dev, private->data->main_path,
private->data->main_len,
-private->ddp_comp))
- ret = BIT(0);
- else if (mtk_drm_find_comp_in_ddp(dev, private->data->ext_path,
- private->data->ext_len,
private->ddp_comp))
- ret = BIT(1);
- else if (mtk_drm_find_comp_in_ddp(dev, private->data->third_path,
- private->data->third_len,
private->ddp_comp))
- ret = BIT(2);
- else
- DRM_INFO("Failed to find comp in ddp table\n");
+ const struct mtk_mmsys_driver_data *data;
+ struct mtk_drm_private *priv_n;
+ int i = 0, j;
+
+ for (j = 0; j < private->data->mmsys_dev_num; j++) {
+ priv_n = private->all_drm_private[j];
+ data = priv_n->data;
+
+
Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
Hi,
My current guess would be that the issue I was seeing was already fixed
with dd9e329af723 ("drm/bridge: ti-sn65dsi83: Fix enable/disable flow
to
meet spec") and I didn't properly test both changes separately.
I had the exact same thought, as I've found your second patch.
My cheap scope is not able to capture the DSI signals and I admit that
we didn't use our more expensive equipment to verify the changes back
then.
Instead, we had an automated test setup to do cyclic on/off switching
for the display and check for a black screen using a sensor. It is
quite
a hassle to set up and I'm currently not planning to spend that much
effort to verify this change again.
That is actually, what we are also doing right now and how the issue was
found in the first place.
Anyway, I currently don't see any reasons to not revert my changes.
Your
revert looks correct and seems to work fine as far as I can tell.
Reviewed-by: Frieder Schrempf
Thanks!
-michael
Re: [PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
Hi Alexander,
The FORCE_STOP_STATE bit is unsuitable to force the DSI link into
LP-11
mode. It seems the bridge internally queues DSI packets and when the
FORCE_STOP_STATE bit is cleared, they are sent in close succession
without any useful timing (this also means that the DSI lanes won't go
into LP-11 mode). The length of this gibberish varies between 1ms and
5ms. This sometimes breaks an attached bridge (TI SN65DSI84 in this
case). In our case, the bridge will fail in about 1 per 500 reboots.
The FORCE_STOP_STATE handling was introduced to have the DSI lanes in
LP-11 state during the .pre_enable phase. But as it turns out, none of
this is needed at all. Between samsung_dsim_init() and
samsung_dsim_set_display_enable() the lanes are already in LP-11 mode.
Apparently LP-11 is actually entered with the call to
samsung_dsim_enable_lane(), but I don't know about other requisites on
that
matter. Unfortunately documentation lacks a lot in that regard.
Which is called during samsung_dsim_atomic_pre_enable(). So I'm not sure
why the FORCE_STOP_STATE was needed at all. Lanes will be in LP-11 mode
if the video stream isn't enabled.
The code as it was before commit 20c827683de0 ("drm: bridge:
samsung-dsim: Fix init during host transfer") and 0c14d3130654 ("drm:
bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec") was
correct
in this regard.
This patch basically reverts both commits. It was tested on an i.MX8M
SoC with an SN65DSI84 bridge. The signals were probed and the DSI
packets were decoded during initialization and link start-up. After
this
patch the first DSI packet on the link is a VSYNC packet and the
timing
is correct.
At which point does SN65DSI84 require LP-11?
I guess I have the same requirement as Frieder (we use the same bridge).
According to the datasheet, the DSI data must be in LP-11 when releasing
EN. According to the init sequence:
- asserting EN
- configure CSRs
- enable video stream
Although not, stated explicitly, LP-11 should also be active during CSR
writes.
But after all, the DSIM driver should adhere to the linux requirement,
which Frieder cited [1].
You have access to a DSI/D-PHY analyzer?
A pretty fast oscilloscope with an DSI decoder. So we can analyze one
DSI
lane.
Command mode between .pre_enable and .enable was also briefly tested
by
a quick hack. There was no DSI link partner which would have
responded,
but it was made sure the DSI packet was send on the link. As a side
note, the command mode seems to just work in HS mode. I couldn't find
that the bridge will handle commands in LP mode.
AFAIK ti-sn65dsi83.c only uses I2C for communication. Did you send DSI
read/
writes instead?
Yeah the bridge only supports I2C. I just wanted to try that a DSI
command
will still work after this patch. As a quick hack, I just added an
mipi_dsi_generic_write() to sn65dsi83_atomic_pre_enable() and made sure
there is a DSI write packet on the actual link.
-michael
best regards,
Alexander
Fixes: 20c827683de0 ("drm: bridge: samsung-dsim: Fix init during host
transfer") Fixes: 0c14d3130654 ("drm: bridge: samsung-dsim: Fix i.MX8M
enable flow to meet spec") Signed-off-by: Michael Walle
---
Let me know wether this should be two commits each reverting one, but
both
commits appeared first in kernel 6.5.
[1]
https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
[PATCH] drm: bridge: samsung-dsim: Don't use FORCE_STOP_STATE
The FORCE_STOP_STATE bit is unsuitable to force the DSI link into LP-11
mode. It seems the bridge internally queues DSI packets and when the
FORCE_STOP_STATE bit is cleared, they are sent in close succession
without any useful timing (this also means that the DSI lanes won't go
into LP-11 mode). The length of this gibberish varies between 1ms and
5ms. This sometimes breaks an attached bridge (TI SN65DSI84 in this
case). In our case, the bridge will fail in about 1 per 500 reboots.
The FORCE_STOP_STATE handling was introduced to have the DSI lanes in
LP-11 state during the .pre_enable phase. But as it turns out, none of
this is needed at all. Between samsung_dsim_init() and
samsung_dsim_set_display_enable() the lanes are already in LP-11 mode.
The code as it was before commit 20c827683de0 ("drm: bridge:
samsung-dsim: Fix init during host transfer") and 0c14d3130654 ("drm:
bridge: samsung-dsim: Fix i.MX8M enable flow to meet spec") was correct
in this regard.
This patch basically reverts both commits. It was tested on an i.MX8M
SoC with an SN65DSI84 bridge. The signals were probed and the DSI
packets were decoded during initialization and link start-up. After this
patch the first DSI packet on the link is a VSYNC packet and the timing
is correct.
Command mode between .pre_enable and .enable was also briefly tested by
a quick hack. There was no DSI link partner which would have responded,
but it was made sure the DSI packet was send on the link. As a side
note, the command mode seems to just work in HS mode. I couldn't find
that the bridge will handle commands in LP mode.
Fixes: 20c827683de0 ("drm: bridge: samsung-dsim: Fix init during host transfer")
Fixes: 0c14d3130654 ("drm: bridge: samsung-dsim: Fix i.MX8M enable flow to meet
spec")
Signed-off-by: Michael Walle
---
Let me know wether this should be two commits each reverting one, but both
commits appeared first in kernel 6.5.
drivers/gpu/drm/bridge/samsung-dsim.c | 32 ++-
1 file changed, 2 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
b/drivers/gpu/drm/bridge/samsung-dsim.c
index cf777bdb25d2..4233a50baac7 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -939,10 +939,6 @@ static int samsung_dsim_init_link(struct samsung_dsim *dsi)
reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
reg &= ~DSIM_STOP_STATE_CNT_MASK;
reg |= DSIM_STOP_STATE_CNT(driver_data->reg_values[STOP_STATE_CNT]);
-
- if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
- reg |= DSIM_FORCE_STOP_STATE;
-
samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
reg = DSIM_BTA_TIMEOUT(0xff) | DSIM_LPDR_TIMEOUT(0x);
@@ -1387,18 +1383,6 @@ static void samsung_dsim_disable_irq(struct samsung_dsim
*dsi)
disable_irq(dsi->irq);
}
-static void samsung_dsim_set_stop_state(struct samsung_dsim *dsi, bool enable)
-{
- u32 reg = samsung_dsim_read(dsi, DSIM_ESCMODE_REG);
-
- if (enable)
- reg |= DSIM_FORCE_STOP_STATE;
- else
- reg &= ~DSIM_FORCE_STOP_STATE;
-
- samsung_dsim_write(dsi, DSIM_ESCMODE_REG, reg);
-}
-
static int samsung_dsim_init(struct samsung_dsim *dsi)
{
const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
@@ -1448,9 +1432,6 @@ static void samsung_dsim_atomic_pre_enable(struct
drm_bridge *bridge,
ret = samsung_dsim_init(dsi);
if (ret)
return;
-
- samsung_dsim_set_display_mode(dsi);
- samsung_dsim_set_display_enable(dsi, true);
}
}
@@ -1459,12 +1440,8 @@ static void samsung_dsim_atomic_enable(struct drm_bridge
*bridge,
{
struct samsung_dsim *dsi = bridge_to_dsi(bridge);
- if (samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
- samsung_dsim_set_display_mode(dsi);
- samsung_dsim_set_display_enable(dsi, true);
- } else {
- samsung_dsim_set_stop_state(dsi, false);
- }
+ samsung_dsim_set_display_mode(dsi);
+ samsung_dsim_set_display_enable(dsi, true);
dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
}
@@ -1477,9 +1454,6 @@ static void samsung_dsim_atomic_disable(struct drm_bridge
*bridge,
if (!(dsi->state & DSIM_STATE_ENABLED))
return;
- if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
- samsung_dsim_set_stop_state(dsi, true);
-
dsi->state &= ~DSIM_STATE_VIDOUT_AVAILABLE;
}
@@ -1781,8 +1755,6 @@ static ssize_t samsung_dsim_host_transfer(struct
mipi_dsi_host *host,
if (ret)
return ret;
- samsung_dsim_set_stop_state(dsi, false);
-
ret = mipi_dsi_create_packet(, msg);
if (ret < 0)
return ret;
--
2.39.2
Re: [PATCH] drm: mediatek: mtk_dsi: Fix NO_EOT_PACKET settings/handling
> Due to the initial confusion about MIPI_DSI_MODE_EOT_PACKET, properly
> renamed to MIPI_DSI_MODE_NO_EOT_PACKET, reflecting its actual meaning,
> both the DSI_TXRX_CON register setting for bit (HSTX_)DIS_EOT and the
> later calculation for horizontal sync-active (HSA), back (HBP) and
> front (HFP) porches got incorrect due to the logic being inverted.
>
> This means that a number of settings were wrong because:
> - DSI_TXRX_CON register setting: bit (HSTX_)DIS_EOT should be
>set in order to disable the End of Transmission packet;
> - Horizontal Sync and Back/Front porches: The delta used to
>calculate all of HSA, HBP and HFP should account for the
>additional EOT packet.
>
> Before this change...
> - Bit (HSTX_)DIS_EOT was being set when EOT packet was enabled;
> - For HSA/HBP/HFP delta... all three were wrong, as words were
>added when EOT disabled, instead of when EOT packet enabled!
>
> Invert the logic around flag MIPI_DSI_MODE_NO_EOT_PACKET in the
> MediaTek DSI driver to fix the aforementioned issues.
>
> Fixes: 8b2b99fd7931 ("drm/mediatek: dsi: Fine tune the line time caused by
> EOTp")
> Fixes: 2d52bfba09d1 ("drm/mediatek: add non-continuous clock mode and EOT
> packet control")
> Signed-off-by: AngeloGioacchino Del Regno
>
Tested-by: Michael Walle
Thanks,
-michael
Re: [PATCH] drm/mediatek: dsi: Fix EOTp generation
Hi,
Am 2023-09-15 10:58, schrieb AngeloGioacchino Del Regno:
Il 15/09/23 09:57, Michael Walle ha scritto:
The commit c87d1c4b5b9a ("drm/mediatek: dsi: Use symbolized register
definition") inverted the logic of the control bit. Maybe it was
because
of the bad naming which was fixed in commit 0f3b68b66a6d ("drm/dsi:
Add
_NO_ to MIPI_DSI_* flags disabling features"). In any case, the logic
wrong and there will be no EOTp on the DSI link by default. Fix it.
Fixes: c87d1c4b5b9a ("drm/mediatek: dsi: Use symbolized register
definition")
Signed-off-by: Michael Walle
Hello Michael,
your commit is missing a small piece! :-)
Besides, I've already sent a fix for what you're trying to do here:
https://lore.kernel.org/linux-arm-kernel/07c93d61-c5fd-f074-abb2-73fdaa81f...@collabora.com/T/
Ahh thanks, didn't noticed this. If not already applied, I'll send
a Tested-by: later.
Please disregard this patch then.
-michael
[PATCH] drm/mediatek: dsi: Fix EOTp generation
The commit c87d1c4b5b9a ("drm/mediatek: dsi: Use symbolized register
definition") inverted the logic of the control bit. Maybe it was because
of the bad naming which was fixed in commit 0f3b68b66a6d ("drm/dsi: Add
_NO_ to MIPI_DSI_* flags disabling features"). In any case, the logic
wrong and there will be no EOTp on the DSI link by default. Fix it.
Fixes: c87d1c4b5b9a ("drm/mediatek: dsi: Use symbolized register definition")
Signed-off-by: Michael Walle
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c
b/drivers/gpu/drm/mediatek/mtk_dsi.c
index d67e5c61a9b9..8024b20f6b13 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -407,7 +407,7 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
if (dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS)
tmp_reg |= HSTX_CKLP_EN;
- if (!(dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET))
+ if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)
tmp_reg |= DIS_EOT;
writel(tmp_reg, dsi->regs + DSI_TXRX_CTRL);
--
2.39.2
[PATCH v4 2/2] drm/mediatek: dpi/dsi: fix possible_crtcs calculation
mtk_drm_find_possible_crtc_by_comp() assumed that the main path will
always have the CRTC with id 0, the ext id 1 and the third id 2. This
is only true if the paths are all available. But paths are optional (see
also comment in mtk_drm_kms_init()), e.g. the main path might not be
enabled or available at all. Then the CRTC IDs will shift one up, e.g.
ext will be 0 and the third path will be 1.
To fix that, dynamically calculate the IDs by the presence of the paths.
While at it, make the return code a signed one and return -ENOENT if no
path is found and handle the error in the callers.
Fixes: 5aa8e7647676 ("drm/mediatek: dpi/dsi: Change the getting possible_crtc
way")
Suggested-by: Nícolas F. R. A. Prado
Signed-off-by: Michael Walle
Reviewed-by: Nícolas F. R. A. Prado
Tested-by: Nícolas F. R. A. Prado
---
v4:
- return -ENOENT if mtk_drm_find_possible_crtc_by_comp() doesn't find
any path
v3:
- use data instead of priv_n->data
- fixed typos
- collected Rb and Tb tags
v2:
- iterate over all_drm_private[] to get any vdosys
- new check if a path is available
---
drivers/gpu/drm/mediatek/mtk_dpi.c | 5 +-
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 75 -
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 3 +-
drivers/gpu/drm/mediatek/mtk_dsi.c | 5 +-
4 files changed, 68 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c
b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 2f931e4e2b60..f9250f7ee706 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -796,7 +796,10 @@ static int mtk_dpi_bind(struct device *dev, struct device
*master, void *data)
return ret;
}
- dpi->encoder.possible_crtcs =
mtk_drm_find_possible_crtc_by_comp(drm_dev, dpi->dev);
+ ret = mtk_drm_find_possible_crtc_by_comp(drm_dev, dpi->dev);
+ if (ret < 0)
+ goto err_cleanup;
+ dpi->encoder.possible_crtcs = ret;
ret = drm_bridge_attach(>encoder, >bridge, NULL,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 771f4e173353..83ae75ecd858 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -507,6 +507,27 @@ static bool mtk_drm_find_comp_in_ddp(struct device *dev,
return false;
}
+static bool mtk_ddp_path_available(const unsigned int *path,
+ unsigned int path_len,
+ struct device_node **comp_node)
+{
+ unsigned int i;
+
+ if (!path)
+ return false;
+
+ for (i = 0U; i < path_len; i++) {
+ /* OVL_ADAPTOR doesn't have a device node */
+ if (path[i] == DDP_COMPONENT_DRM_OVL_ADAPTOR)
+ continue;
+
+ if (!comp_node[path[i]])
+ return false;
+ }
+
+ return true;
+}
+
int mtk_ddp_comp_get_id(struct device_node *node,
enum mtk_ddp_comp_type comp_type)
{
@@ -522,25 +543,47 @@ int mtk_ddp_comp_get_id(struct device_node *node,
return -EINVAL;
}
-unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm,
- struct device *dev)
+int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm, struct device
*dev)
{
struct mtk_drm_private *private = drm->dev_private;
- unsigned int ret = 0;
-
- if (mtk_drm_find_comp_in_ddp(dev, private->data->main_path,
private->data->main_len,
-private->ddp_comp))
- ret = BIT(0);
- else if (mtk_drm_find_comp_in_ddp(dev, private->data->ext_path,
- private->data->ext_len,
private->ddp_comp))
- ret = BIT(1);
- else if (mtk_drm_find_comp_in_ddp(dev, private->data->third_path,
- private->data->third_len,
private->ddp_comp))
- ret = BIT(2);
- else
- DRM_INFO("Failed to find comp in ddp table\n");
+ const struct mtk_mmsys_driver_data *data;
+ struct mtk_drm_private *priv_n;
+ int i = 0, j;
+
+ for (j = 0; j < private->data->mmsys_dev_num; j++) {
+ priv_n = private->all_drm_private[j];
+ data = priv_n->data;
+
+ if (mtk_ddp_path_available(data->main_path, data->main_len,
+ priv_n->comp_node)) {
+ if (mtk_drm_find_comp_in_ddp(dev, data->main_path,
+data->main_len,
+priv_n->ddp_comp))
+
[PATCH v4 1/2] drm/mediatek: fix kernel oops if no crtc is found
drm_crtc_from_index(0) might return NULL if there are no CRTCs
registered at all which will lead to a kernel oops in
mtk_drm_crtc_dma_dev_get(). Add the missing return value check.
Fixes: 0d9eee9118b7 ("drm/mediatek: Add drm ovl_adaptor sub driver for MT8195")
Signed-off-by: Michael Walle
Reviewed-by: Nícolas F. R. A. Prado
Tested-by: Nícolas F. R. A. Prado
Reviewed-by: AngeloGioacchino Del Regno
---
v4:
- collected tags
v3:
- none
v2:
- collected tags
- fixed typos
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 93552d76b6e7..2c582498817e 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -420,6 +420,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
struct mtk_drm_private *private = drm->dev_private;
struct mtk_drm_private *priv_n;
struct device *dma_dev = NULL;
+ struct drm_crtc *crtc;
int ret, i, j;
if (drm_firmware_drivers_only())
@@ -494,7 +495,9 @@ static int mtk_drm_kms_init(struct drm_device *drm)
}
/* Use OVL device for all DMA memory allocations */
- dma_dev = mtk_drm_crtc_dma_dev_get(drm_crtc_from_index(drm, 0));
+ crtc = drm_crtc_from_index(drm, 0);
+ if (crtc)
+ dma_dev = mtk_drm_crtc_dma_dev_get(crtc);
if (!dma_dev) {
ret = -ENODEV;
dev_err(drm->dev, "Need at least one OVL device\n");
--
2.39.2
Re: [PATCH v3 2/2] drm/mediatek: dpi/dsi: fix possible_crtcs calculation
At this point, I think that it would be sane to change this function to return a signed type, so that we can return -ENOENT if we couldn't find any DDP path (so, if we couldn't find any possible crtc). Fair enough, but should it be part of the fixes commit or a different one? -michael
[PATCH v3 2/2] drm/mediatek: dpi/dsi: fix possible_crtcs calculation
mtk_drm_find_possible_crtc_by_comp() assumed that the main path will
always have the CRTC with id 0, the ext id 1 and the third id 2. This
is only true if the paths are all available. But paths are optional (see
also comment in mtk_drm_kms_init()), e.g. the main path might not be
enabled or available at all. Then the CRTC IDs will shift one up, e.g.
ext will be 0 and the third path will be 1.
To fix that, dynamically calculate the IDs by the presence of the paths.
Fixes: 5aa8e7647676 ("drm/mediatek: dpi/dsi: Change the getting possible_crtc
way")
Suggested-by: Nícolas F. R. A. Prado
Signed-off-by: Michael Walle
Reviewed-by: Nícolas F. R. A. Prado
Tested-by: Nícolas F. R. A. Prado
---
v3:
- use data instead of priv_n->data
- fixed typos
- collected Rb and Tb tags
v2:
- iterate over all_drm_private[] to get any vdosys
- new check if a path is available
---
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 72 +
1 file changed, 58 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 771f4e173353..c00f4669cc50 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -507,6 +507,27 @@ static bool mtk_drm_find_comp_in_ddp(struct device *dev,
return false;
}
+static bool mtk_ddp_path_available(const unsigned int *path,
+ unsigned int path_len,
+ struct device_node **comp_node)
+{
+ unsigned int i;
+
+ if (!path)
+ return false;
+
+ for (i = 0U; i < path_len; i++) {
+ /* OVL_ADAPTOR doesn't have a device node */
+ if (path[i] == DDP_COMPONENT_DRM_OVL_ADAPTOR)
+ continue;
+
+ if (!comp_node[path[i]])
+ return false;
+ }
+
+ return true;
+}
+
int mtk_ddp_comp_get_id(struct device_node *node,
enum mtk_ddp_comp_type comp_type)
{
@@ -526,21 +547,44 @@ unsigned int mtk_drm_find_possible_crtc_by_comp(struct
drm_device *drm,
struct device *dev)
{
struct mtk_drm_private *private = drm->dev_private;
- unsigned int ret = 0;
-
- if (mtk_drm_find_comp_in_ddp(dev, private->data->main_path,
private->data->main_len,
-private->ddp_comp))
- ret = BIT(0);
- else if (mtk_drm_find_comp_in_ddp(dev, private->data->ext_path,
- private->data->ext_len,
private->ddp_comp))
- ret = BIT(1);
- else if (mtk_drm_find_comp_in_ddp(dev, private->data->third_path,
- private->data->third_len,
private->ddp_comp))
- ret = BIT(2);
- else
- DRM_INFO("Failed to find comp in ddp table\n");
+ const struct mtk_mmsys_driver_data *data;
+ struct mtk_drm_private *priv_n;
+ int i = 0, j;
+
+ for (j = 0; j < private->data->mmsys_dev_num; j++) {
+ priv_n = private->all_drm_private[j];
+ data = priv_n->data;
+
+ if (mtk_ddp_path_available(data->main_path, data->main_len,
+ priv_n->comp_node)) {
+ if (mtk_drm_find_comp_in_ddp(dev, data->main_path,
+data->main_len,
+priv_n->ddp_comp))
+ return BIT(i);
+ i++;
+ }
+
+ if (mtk_ddp_path_available(data->ext_path, data->ext_len,
+ priv_n->comp_node)) {
+ if (mtk_drm_find_comp_in_ddp(dev, data->ext_path,
+data->ext_len,
+priv_n->ddp_comp))
+ return BIT(i);
+ i++;
+ }
+
+ if (mtk_ddp_path_available(data->third_path, data->third_len,
+ priv_n->comp_node)) {
+ if (mtk_drm_find_comp_in_ddp(dev, data->third_path,
+data->third_len,
+priv_n->ddp_comp))
+ return BIT(i);
+ i++;
+ }
+ }
- return ret;
+ DRM_INFO("Failed to find comp in ddp table\n");
+ return 0;
}
int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp,
--
2.39.2
[PATCH v3 1/2] drm/mediatek: fix kernel oops if no crtc is found
drm_crtc_from_index(0) might return NULL if there are no CRTCs
registered at all which will lead to a kernel oops in
mtk_drm_crtc_dma_dev_get(). Add the missing return value check.
Fixes: 0d9eee9118b7 ("drm/mediatek: Add drm ovl_adaptor sub driver for MT8195")
Signed-off-by: Michael Walle
Reviewed-by: Nícolas F. R. A. Prado
Tested-by: Nícolas F. R. A. Prado
---
v3:
- none
v2:
- collected tags
- fixed typos
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 93552d76b6e7..2c582498817e 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -420,6 +420,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
struct mtk_drm_private *private = drm->dev_private;
struct mtk_drm_private *priv_n;
struct device *dma_dev = NULL;
+ struct drm_crtc *crtc;
int ret, i, j;
if (drm_firmware_drivers_only())
@@ -494,7 +495,9 @@ static int mtk_drm_kms_init(struct drm_device *drm)
}
/* Use OVL device for all DMA memory allocations */
- dma_dev = mtk_drm_crtc_dma_dev_get(drm_crtc_from_index(drm, 0));
+ crtc = drm_crtc_from_index(drm, 0);
+ if (crtc)
+ dma_dev = mtk_drm_crtc_dma_dev_get(crtc);
if (!dma_dev) {
ret = -ENODEV;
dev_err(drm->dev, "Need at least one OVL device\n");
--
2.39.2
[PATCH v2 1/2] drm/mediatek: fix kernel oops if no crtc is found
drm_crtc_from_index(0) might return NULL if there are no CRTCs
registered at all which will lead to a kernel oops in
mtk_drm_crtc_dma_dev_get(). Add the missing return value check.
Fixes: 0d9eee9118b7 ("drm/mediatek: Add drm ovl_adaptor sub driver for MT8195")
Signed-off-by: Michael Walle
Reviewed-by: Nícolas F. R. A. Prado
Tested-by: Nícolas F. R. A. Prado
---
v2:
- collected tags
- fixed typos
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 93552d76b6e7..2c582498817e 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -420,6 +420,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
struct mtk_drm_private *private = drm->dev_private;
struct mtk_drm_private *priv_n;
struct device *dma_dev = NULL;
+ struct drm_crtc *crtc;
int ret, i, j;
if (drm_firmware_drivers_only())
@@ -494,7 +495,9 @@ static int mtk_drm_kms_init(struct drm_device *drm)
}
/* Use OVL device for all DMA memory allocations */
- dma_dev = mtk_drm_crtc_dma_dev_get(drm_crtc_from_index(drm, 0));
+ crtc = drm_crtc_from_index(drm, 0);
+ if (crtc)
+ dma_dev = mtk_drm_crtc_dma_dev_get(crtc);
if (!dma_dev) {
ret = -ENODEV;
dev_err(drm->dev, "Need at least one OVL device\n");
--
2.39.2
Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus
Hi, I was just curious if you know of any development for that (or similar) in the kernel. This is probably because support for this SoC began with Chromebooks, which have fixed and defined uses for the pipelines. I suspect that what you are working on is much more flexible. Yes. that is correct. The driver should be made to allow dynamic selection of outputs, as is commonly seen with other drivers, but I don't know if that's on anyone's TODO list. Do you have any pointers where to look at? -michael
[PATCH v2 2/2] drm/mediatek: dpi/dsi: fix possible_crtcs calculation
mtk_drm_find_possible_crtc_by_comp() assumed that the main path will
always have the CRTC with id 0, the ext id 1 and the third id 2. This
is only true if the paths are all available. But paths are optional (see
also comment in mtk_drm_kms_init()), e.g. the main path might not be
enabled or available at all. Then the CRTC IDs will shift one up, e.g.
ext will be 1 and the third path will be 2.
To fix that, dynamically calculate the IDs by the precence of the paths.
Fixes: 5aa8e7647676 ("drm/mediatek: dpi/dsi: Change the getting possible_crtc
way")
Suggested-by: Nícolas F. R. A. Prado
Signed-off-by: Michael Walle
---
v2:
- iterate over all_drm_private[] to get any vdosys
- new check if a path is available
---
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 72 +
1 file changed, 58 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 771f4e173353..9f0f12740fb0 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -507,6 +507,27 @@ static bool mtk_drm_find_comp_in_ddp(struct device *dev,
return false;
}
+static bool mtk_ddp_path_available(const unsigned int *path,
+ unsigned int path_len,
+ struct device_node **comp_node)
+{
+ unsigned int i;
+
+ if (!path)
+ return false;
+
+ for (i = 0U; i < path_len; i++) {
+ /* OVL_ADAPTOR doesn't have a device node */
+ if (path[i] == DDP_COMPONENT_DRM_OVL_ADAPTOR)
+ continue;
+
+ if (!comp_node[path[i]])
+ return false;
+ }
+
+ return true;
+}
+
int mtk_ddp_comp_get_id(struct device_node *node,
enum mtk_ddp_comp_type comp_type)
{
@@ -526,21 +547,44 @@ unsigned int mtk_drm_find_possible_crtc_by_comp(struct
drm_device *drm,
struct device *dev)
{
struct mtk_drm_private *private = drm->dev_private;
- unsigned int ret = 0;
-
- if (mtk_drm_find_comp_in_ddp(dev, private->data->main_path,
private->data->main_len,
-private->ddp_comp))
- ret = BIT(0);
- else if (mtk_drm_find_comp_in_ddp(dev, private->data->ext_path,
- private->data->ext_len,
private->ddp_comp))
- ret = BIT(1);
- else if (mtk_drm_find_comp_in_ddp(dev, private->data->third_path,
- private->data->third_len,
private->ddp_comp))
- ret = BIT(2);
- else
- DRM_INFO("Failed to find comp in ddp table\n");
+ const struct mtk_mmsys_driver_data *data;
+ struct mtk_drm_private *priv_n;
+ int i = 0, j;
+
+ for (j = 0; j < private->data->mmsys_dev_num; j++) {
+ priv_n = private->all_drm_private[j];
+ data = priv_n->data;
+
+ if (mtk_ddp_path_available(data->main_path, data->main_len,
+ priv_n->comp_node)) {
+ if (mtk_drm_find_comp_in_ddp(dev,
priv_n->data->main_path,
+priv_n->data->main_len,
+priv_n->ddp_comp))
+ return BIT(i);
+ i++;
+ }
+
+ if (mtk_ddp_path_available(data->ext_path, data->ext_len,
+ priv_n->comp_node)) {
+ if (mtk_drm_find_comp_in_ddp(dev,
priv_n->data->ext_path,
+priv_n->data->ext_len,
+priv_n->ddp_comp))
+ return BIT(i);
+ i++;
+ }
+
+ if (mtk_ddp_path_available(data->third_path, data->third_len,
+ priv_n->comp_node)) {
+ if (mtk_drm_find_comp_in_ddp(dev,
priv_n->data->third_path,
+priv_n->data->third_len,
+priv_n->ddp_comp))
+ return BIT(i);
+ i++;
+ }
+ }
- return ret;
+ DRM_INFO("Failed to find comp in ddp table\n");
+ return 0;
}
int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp,
--
2.39.2
Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus
While digging through the code I realized that all the outputs and pipelines are harcoded. Doh. For all the mediatek SoCs. Looks like major restriction to me. E.g. there is also DSI and HDMI output on the mt8195. I looked at the downstream linux and there, the output is not part of the pipeline. Are you aware of any work in that direction? I'm not sure I get what output and pipelines are hardcoded that you're referring to (besides the one in the mtk-dsi/dpi driver that you already sent the patch fixing). Have a look at [1]. That main path ends with the DP_INTF0 which is the eDP output. But from what I understand that path can also use the DSI output. But that is a pattern for all the paths in that file. Looks like all supported boards in the kernel will have the output type for a given SoC/path (or maybe the mt8195 is the first one which supports different output interfaces). If you have a look at the mediatek kernel instead [2], there the last part of the path is not fixed, but there is also a .conn_routes property by which you seem to be able to choose the actual output interface. I was just curious if you know of any development for that (or similar) in the kernel. -michael And I'm not familiar with the DSI and HDMI output support on MT8195, so I can't help with that. Thanks, Nícolas [1] https://elixir.bootlin.com/linux/v6.5/source/drivers/gpu/drm/mediatek/mtk_drm_drv.c#L210 [2] https://gitlab.com/mediatek/aiot/bsp/linux/-/blob/mtk-v5.15-dev/drivers/gpu/drm/mediatek/mtk_drm_drv.c?ref_type=heads#L425
Re: [PATCH 2/2] drm/mediatek: dpi/dsi: fix possible_crtcs calculation
This won't work. On MT8195 there are two display IPs, vdosys0 and vdosys1, vdosys0 only has the main path while vdosys1 only has the external path. So you need to loop over each one in all_drm_private[j] to get the right crtc ID for MT8195. Ahh thanks, got it. -michael
Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus
Hi Nícolas, But the real reason I've enabled it was because I'll get an kernel oops otherwise. I thought it might be some quirk that you'll need both, because eDP will register even if theres no display - as you've mentioned below. Here's the splat: [3.237064] mediatek-drm mediatek-drm.10.auto: bound 1c11.vpp-merge (ops mtk_disp_merge_component_ops) [3.238274] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0 because component 8 is disabled or missing [3.239504] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0 because component 9 is disabled or missing [3.240741] Unable to handle kernel NULL pointer dereference at virtual address 04a0 [3.241841] Mem abort info: [3.242192] ESR = 0x9604 [3.242662] EC = 0x25: DABT (current EL), IL = 32 bits [3.243328] SET = 0, FnV = 0 [3.243710] EA = 0, S1PTW = 0 [3.244104] FSC = 0x04: level 0 translation fault [3.244717] Data abort info: [3.245078] ISV = 0, ISS = 0x0004, ISS2 = 0x [3.245765] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 [3.246398] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [3.247063] [04a0] user address but active_mm is swapper [3.247860] Internal error: Oops: 9604 [#1] SMP [3.248559] Modules linked in: [3.248945] CPU: 4 PID: 11 Comm: kworker/u16:0 Not tainted 6.5.0-rc7-next-20230821+ #2225 [3.249970] Hardware name: Kontron 3.5"-SBC-i1200 (DT) [3.250614] Workqueue: events_unbound deferred_probe_work_func [3.251347] pstate: 6049 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [3.252220] pc : mtk_drm_crtc_dma_dev_get+0x8/0x18 [3.252824] lr : mtk_drm_bind+0x458/0x558 [3.253326] sp : 800082b23a20 [3.253741] x29: 800082b23a20 x28: 02c78880 x27: 8000816466d0 [3.254635] x26: 02c6f010 x25: 0003 x24: [3.255529] x23: 02c78880 x22: 0002 x21: [3.256423] x20: 06516800 x19: 02c78880 x18: [3.257317] x17: 6f63206573756163 x16: 6562203020637472 x15: 6320676e69746165 [3.258211] x14: 726320746f4e203a x13: 676e697373696d20 x12: 726f2064656c6261 [3.259106] x11: 7369642073692039 x10: 80008275c0c0 x9 : 80008091ebf8 [3.26] x8 : efff x7 : 80008275c0c0 x6 : 8000f000 [3.260895] x5 : bff4 x4 : x3 : 06516ae0 [3.261789] x2 : 06516ae0 x1 : x0 : [3.262684] Call trace: [3.262991] mtk_drm_crtc_dma_dev_get+0x8/0x18 [3.263549] try_to_bring_up_aggregate_device+0x16c/0x1e0 [3.264227] __component_add+0xac/0x180 [3.264708] component_add+0x1c/0x30 [3.265158] mtk_disp_rdma_probe+0x17c/0x270 [3.265695] platform_probe+0x70/0xd0 [3.266155] really_probe+0x150/0x2c0 [3.266615] __driver_probe_device+0x80/0x140 [3.267162] driver_probe_device+0x44/0x170 [3.267687] __device_attach_driver+0xc0/0x148 [3.268245] bus_for_each_drv+0x88/0xf0 [3.268727] __device_attach+0xa4/0x198 [3.269208] device_initial_probe+0x1c/0x30 [3.269732] bus_probe_device+0xb4/0xc0 [3.270214] deferred_probe_work_func+0x90/0xd0 [3.270783] process_one_work+0x144/0x3a0 [3.271289] worker_thread+0x2ac/0x4b8 [3.271761] kthread+0xec/0xf8 [3.272145] ret_from_fork+0x10/0x20 [3.272597] Code: 814f7858 8000 aa1e03e9 d503201f (f9425000) [3.273361] ---[ end trace ]--- I tried reproducing this on mt8192-asurada-spherion and mt8195-cherry-tomato but wasn't able to. However, I did see another issue Yeah sorry, I tried to reproduce my initial oops but messed my DT up and ended up with no path enabled at all. [3.183314] mediatek-drm mediatek-drm.9.auto: Not creating crtc 0 because component 14 is disabled or missing [3.199404] Bogus possible_crtcs: [ENCODER:31:TMDS-31] possible_crtcs=0x2 (full crtc mask=0x1) [3.208081] WARNING: CPU: 6 PID: 68 at drivers/gpu/drm/drm_mode_config.c:626 drm_mode_config_validate+0x1c8/0x548 [3.224789] Modules linked in: [3.227838] CPU: 6 PID: 68 Comm: kworker/u16:1 Not tainted 6.5.0-rc3-next-20230728+ #100 [3.235918] Hardware name: Google Spherion (rev0 - 3) (DT) [3.241391] Workqueue: events_unbound deferred_probe_work_func [3.247216] pstate: 6049 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [3.254167] pc : drm_mode_config_validate+0x1c8/0x548 [3.259209] lr : drm_mode_config_validate+0x1c8/0x548 [3.264250] sp : 8000804e3970 [3.267552] x29: 8000804e3980 x28: 4841827c1880 x27: 0001 [3.274677] x26: 0001 x25: 4841825f5ab0 x24: 4841825f5ab0 [3.281801] x23: 484182469880 x22: a80dbfbdde28 x21: a80dbfbddba8 [3.288925] x20: 4841825f5800 x19: 4841825f5aa8 x18: 0030 [3.296050]
[PATCH 2/2] drm/mediatek: dpi/dsi: fix possible_crtcs calculation
mtk_drm_find_possible_crtc_by_comp() assumed that the main path will
always have the CRTC with id 0, the ext id 1 and the third id 2. This
is only true if the paths are all available. But paths are optional (see
also comment in mtk_drm_kms_init()), e.g. the main path might not be
enabled or available at all. Then the CRTC IDs will shift one up, e.g.
ext will be 1 and the third path will be 2.
To fix that, dynamically calculate the IDs by the precence of the paths.
Fixes: 5aa8e7647676 ("drm/mediatek: dpi/dsi: Change the getting possible_crtc
way")
Suggested-by: Nícolas F. R. A. Prado
Signed-off-by: Michael Walle
---
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 41 ++---
1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 771f4e173353..f3064bff64e8 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -526,21 +526,34 @@ unsigned int mtk_drm_find_possible_crtc_by_comp(struct
drm_device *drm,
struct device *dev)
{
struct mtk_drm_private *private = drm->dev_private;
- unsigned int ret = 0;
-
- if (mtk_drm_find_comp_in_ddp(dev, private->data->main_path,
private->data->main_len,
-private->ddp_comp))
- ret = BIT(0);
- else if (mtk_drm_find_comp_in_ddp(dev, private->data->ext_path,
- private->data->ext_len,
private->ddp_comp))
- ret = BIT(1);
- else if (mtk_drm_find_comp_in_ddp(dev, private->data->third_path,
- private->data->third_len,
private->ddp_comp))
- ret = BIT(2);
- else
- DRM_INFO("Failed to find comp in ddp table\n");
+ int i = 0;
+
+ if (private->data->main_path) {
+ if (mtk_drm_find_comp_in_ddp(dev, private->data->main_path,
+private->data->main_len,
+private->ddp_comp))
+ return BIT(i);
+ i++;
+ }
+
+ if (private->data->ext_path) {
+ if (mtk_drm_find_comp_in_ddp(dev, private->data->ext_path,
+private->data->ext_len,
+private->ddp_comp))
+ return BIT(i);
+ i++;
+ }
- return ret;
+ if (private->data->third_path) {
+ if (mtk_drm_find_comp_in_ddp(dev, private->data->third_path,
+private->data->third_len,
+private->ddp_comp))
+ return BIT(i);
+ i++;
+ }
+
+ DRM_INFO("Failed to find comp in ddp table\n");
+ return 0;
}
int mtk_ddp_comp_init(struct device_node *node, struct mtk_ddp_comp *comp,
--
2.39.2
[PATCH 1/2] drm/mediathek: fix kernel oops if no crtc is found
drm_crtc_from_index(0) might return NULL if there are not CRTCs
registered at all which will lead to a kernel oops in
mtk_drm_crtc_dma_dev_get(). Add the missing return value check.
Fixes: 0d9eee9118b7 ("drm/mediatek: Add drm ovl_adaptor sub driver for MT8195")
Signed-off-by: Michael Walle
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 93552d76b6e7..2c582498817e 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -420,6 +420,7 @@ static int mtk_drm_kms_init(struct drm_device *drm)
struct mtk_drm_private *private = drm->dev_private;
struct mtk_drm_private *priv_n;
struct device *dma_dev = NULL;
+ struct drm_crtc *crtc;
int ret, i, j;
if (drm_firmware_drivers_only())
@@ -494,7 +495,9 @@ static int mtk_drm_kms_init(struct drm_device *drm)
}
/* Use OVL device for all DMA memory allocations */
- dma_dev = mtk_drm_crtc_dma_dev_get(drm_crtc_from_index(drm, 0));
+ crtc = drm_crtc_from_index(drm, 0);
+ if (crtc)
+ dma_dev = mtk_drm_crtc_dma_dev_get(crtc);
if (!dma_dev) {
ret = -ENODEV;
dev_err(drm->dev, "Need at least one OVL device\n");
--
2.39.2
Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus
Hi Nicolas, > For the eDP case we can support using aux-bus on MediaTek DP: this > gives us the possibility to declare our panel as generic "panel-edp" > which will automatically configure the timings and available modes > via the EDID that we read from it. > > To do this, move the panel parsing at the end of the probe function > so that the hardware is initialized beforehand and also initialize > the DPTX AUX block and power both on as, when we populate the > aux-bus, the panel driver will trigger an EDID read to perform > panel detection. > > Last but not least, since now the AUX transfers can happen in the > separated aux-bus, it was necessary to add an exclusion for the > cable_plugged_in check in `mtk_dp_aux_transfer()` and the easiest > way to do this is to simply ignore checking that when the bridge > type is eDP. This patch breaks my board based on the MT8195 which only has one DisplayPort output port. I suspect it might also break the mt8195-cherry board. Do you mean that your board does not have an internal display, only the one output port? If so, why are you enabling the nodes for the internal display path in your board specific DT? Well, that depends. The board actually has an eDP socket, but because we are an OEM, there might be no display connected to it. (And I haven't tried it yet). It should probably go into an own device tree or overlay if it is used. I agree. But it looked like it was auto-detectable (it even has a HDP pin on the eDP socket, not sure about its use case..) But the real reason I've enabled it was because I'll get an kernel oops otherwise. I thought it might be some quirk that you'll need both, because eDP will register even if theres no display - as you've mentioned below. Here's the splat: [3.237064] mediatek-drm mediatek-drm.10.auto: bound 1c11.vpp-merge (ops mtk_disp_merge_component_ops) [3.238274] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0 because component 8 is disabled or missing [3.239504] mediatek-drm mediatek-drm.8.auto: Not creating crtc 0 because component 9 is disabled or missing [3.240741] Unable to handle kernel NULL pointer dereference at virtual address 04a0 [3.241841] Mem abort info: [3.242192] ESR = 0x9604 [3.242662] EC = 0x25: DABT (current EL), IL = 32 bits [3.243328] SET = 0, FnV = 0 [3.243710] EA = 0, S1PTW = 0 [3.244104] FSC = 0x04: level 0 translation fault [3.244717] Data abort info: [3.245078] ISV = 0, ISS = 0x0004, ISS2 = 0x [3.245765] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 [3.246398] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [3.247063] [04a0] user address but active_mm is swapper [3.247860] Internal error: Oops: 9604 [#1] SMP [3.248559] Modules linked in: [3.248945] CPU: 4 PID: 11 Comm: kworker/u16:0 Not tainted 6.5.0-rc7-next-20230821+ #2225 [3.249970] Hardware name: Kontron 3.5"-SBC-i1200 (DT) [3.250614] Workqueue: events_unbound deferred_probe_work_func [3.251347] pstate: 6049 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [3.252220] pc : mtk_drm_crtc_dma_dev_get+0x8/0x18 [3.252824] lr : mtk_drm_bind+0x458/0x558 [3.253326] sp : 800082b23a20 [3.253741] x29: 800082b23a20 x28: 02c78880 x27: 8000816466d0 [3.254635] x26: 02c6f010 x25: 0003 x24: [3.255529] x23: 02c78880 x22: 0002 x21: [3.256423] x20: 06516800 x19: 02c78880 x18: [3.257317] x17: 6f63206573756163 x16: 6562203020637472 x15: 6320676e69746165 [3.258211] x14: 726320746f4e203a x13: 676e697373696d20 x12: 726f2064656c6261 [3.259106] x11: 7369642073692039 x10: 80008275c0c0 x9 : 80008091ebf8 [3.26] x8 : efff x7 : 80008275c0c0 x6 : 8000f000 [3.260895] x5 : bff4 x4 : x3 : 06516ae0 [3.261789] x2 : 06516ae0 x1 : x0 : [3.262684] Call trace: [3.262991] mtk_drm_crtc_dma_dev_get+0x8/0x18 [3.263549] try_to_bring_up_aggregate_device+0x16c/0x1e0 [3.264227] __component_add+0xac/0x180 [3.264708] component_add+0x1c/0x30 [3.265158] mtk_disp_rdma_probe+0x17c/0x270 [3.265695] platform_probe+0x70/0xd0 [3.266155] really_probe+0x150/0x2c0 [3.266615] __driver_probe_device+0x80/0x140 [3.267162] driver_probe_device+0x44/0x170 [3.267687] __device_attach_driver+0xc0/0x148 [3.268245] bus_for_each_drv+0x88/0xf0 [3.268727] __device_attach+0xa4/0x198 [3.269208] device_initial_probe+0x1c/0x30 [3.269732] bus_probe_device+0xb4/0xc0 [3.270214] deferred_probe_work_func+0x90/0xd0 [3.270783] process_one_work+0x144/0x3a0 [3.271289] worker_thread+0x2ac/0x4b8 [3.271761] kthread+0xec/0xf8 [3.272145] ret_from_fork+0x10/0x20 [
Re: [PATCH v7 09/11] drm/mediatek: dp: Add support for embedded DisplayPort aux-bus
Hi AngeloGioacchino, > For the eDP case we can support using aux-bus on MediaTek DP: this > gives us the possibility to declare our panel as generic "panel-edp" > which will automatically configure the timings and available modes > via the EDID that we read from it. > > To do this, move the panel parsing at the end of the probe function > so that the hardware is initialized beforehand and also initialize > the DPTX AUX block and power both on as, when we populate the > aux-bus, the panel driver will trigger an EDID read to perform > panel detection. > > Last but not least, since now the AUX transfers can happen in the > separated aux-bus, it was necessary to add an exclusion for the > cable_plugged_in check in `mtk_dp_aux_transfer()` and the easiest > way to do this is to simply ignore checking that when the bridge > type is eDP. This patch breaks my board based on the MT8195 which only has one DisplayPort output port. I suspect it might also break the mt8195-cherry board. While the mediatek-dpi driver finds the DP port: [3.131645] mediatek-dpi 1c113000.dp-intf: Found bridge node: /soc/dp-tx@1c60 The probing of the eDP is deferred: [ 13.289009] platform 1c015000.dp-intf: deferred probe pending So I don't know why, but to make dp_intf1 work, it seems that dp_intf0 must be probed successfully. After this patch, the edp (which is connected to the dp_intf1) probe will return with an -ENODEV and the previous call to devm_drm_bridge_add() will be rolled back. Before this patch, bridge_add() was called in any case (in the error case with next_bridge = NULL) and the mediatek-dpi probed like that: [3.121011] mediatek-dpi 1c015000.dp-intf: Found bridge node: /soc/edp-tx@1c50 [3.122111] mediatek-dpi 1c113000.dp-intf: Found bridge node: /soc/dp-tx@1c60 Eventually resulting in a framebuffer device: [4.451081] mediatek-drm mediatek-drm.8.auto: [drm] fb0: mediatekdrmfb frame buffer device NB, somehow this series broke the initial display output. I always have to replug the DisplayPort to get some output. I'll dig deeper into that later. .. > @@ -2519,21 +2553,14 @@ static int mtk_dp_probe(struct platform_device *pdev) > return dev_err_probe(dev, mtk_dp->irq, >"failed to request dp irq resource\n"); > > - mtk_dp->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0); > - if (IS_ERR(mtk_dp->next_bridge) && > - PTR_ERR(mtk_dp->next_bridge) == -ENODEV) > - mtk_dp->next_bridge = NULL; In my case, this branch was taken. -michael > - else if (IS_ERR(mtk_dp->next_bridge)) > - return dev_err_probe(dev, PTR_ERR(mtk_dp->next_bridge), > - "Failed to get bridge\n"); > - > ret = mtk_dp_dt_parse(mtk_dp, pdev); > if (ret) > return dev_err_probe(dev, ret, "Failed to parse dt\n"); > > - drm_dp_aux_init(_dp->aux); > mtk_dp->aux.name = "aux_mtk_dp"; > + mtk_dp->aux.dev = dev; > mtk_dp->aux.transfer = mtk_dp_aux_transfer; > + drm_dp_aux_init(_dp->aux); > > spin_lock_init(_dp->irq_thread_lock); >
Re: [PATCH v2 1/2] pwm: Manage owner assignment implicitly for drivers
Hi, Instead of requiring each driver to care for assigning the owner member of struct pwm_ops, handle that implicitly using a macro. Note that the owner member has to be moved to struct pwm_chip, as the ops structure usually lives in read-only memory and so cannot be modified. The upside is that new lowlevel drivers cannot forget the assignment and save one line each. The pwm-crc driver didn't assign .owner, that's not a problem in practise though as the driver cannot be compiled as a module. Signed-off-by: Uwe Kleine-König ... drivers/pwm/pwm-sl28cpld.c| 1 - Acked-by: Michael Walle
[PATCH V3 5/7] drm: bridge: samsung-dsim: Dynamically configure DPHY timing
> The DPHY timings are currently hard coded. Since the input > clock can be variable, the phy timings need to be variable > too. Add an additional variable to the driver data to enable > this feature to prevent breaking boards that don't support it. > > The phy_mipi_dphy_get_default_config function configures the > DPHY timings in pico-seconds, and a small macro converts those > timings into clock cycles based on the pixel clock rate. This actually fixes a bug with the DSI84 bridge on our boards. The hardcoded settings will violate the D-PHY spec timings for lower frequencies, esp. the Ths_prepare+Ths_zero timing. Thus, the bridge will read a wrong HS sync sequence and set it's internal SoT error bit (and don't generate any RGB signals on the LVDS side). Tested-by: Michael Walle Thanks! -michael
Re: [PATCH v2] drm/etnaviv: avoid cleaning up sched_job when submit succeeded
Am 2022-05-04 11:02, schrieb Lucas Stach:
While the labels may mislead the casual reader, the tail of the
function
etnaviv_ioctl_gem_submit is always executed, as a lot of the structures
set up in this function need to be cleaned up regardless of whether the
submit succeeded or failed.
An exception is the newly added drm_sched_job_cleanup, which must only
be called when the submit failed before handing the job to the
scheduler.
Fixes: b827c84f5e84 ("drm/etnaviv: Use scheduler dependency handling")
Reported-by: Michael Walle
Signed-off-by: Lucas Stach
FWIW (because it's already picked up)
Tested-by: Michael Walle
Thanks!
-michael
Re: [PATCH 1/4] drm/etnaviv: Use scheduler dependency handling
> We need to pull the drm_sched_job_init much earlier, but that's very > minor surgery. This patch breaks the GC7000 on the LS1028A: [ 35.671102] Unable to handle kernel NULL pointer dereference at virtual address 0078 [ 35.680925] Mem abort info: [ 35.685127] ESR = 0x9604 [ 35.689583] EC = 0x25: DABT (current EL), IL = 32 bits [ 35.696312] SET = 0, FnV = 0 [ 35.700766] EA = 0, S1PTW = 0 [ 35.705315] FSC = 0x04: level 0 translation fault [ 35.711616] Data abort info: [ 35.715916] ISV = 0, ISS = 0x0004 [ 35.721170] CM = 0, WnR = 0 [ 35.725552] user pgtable: 4k pages, 48-bit VAs, pgdp=002083f59000 [ 35.733420] [0078] pgd=, p4d= [ 35.741627] Internal error: Oops: 9604 [#1] SMP [ 35.747902] Modules linked in: [ 35.750963] CPU: 0 PID: 44 Comm: f0c.gpu Not tainted 5.18.0-rc2-00894-gde6a1d7294f5 #24 [ 35.759345] Hardware name: Kontron KBox A-230-LS (DT) [ 35.764409] pstate: 0005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 35.771394] pc : drm_sched_entity_select_rq+0x314/0x380 [ 35.776645] lr : drm_sched_entity_pop_job+0x4c/0x480 [ 35.781625] sp : 8949bdb0 [ 35.784943] x29: 8949bdb0 x28: x27: [ 35.792107] x26: 002003f09008 x25: 00200231d130 x24: 88c13008 [ 35.799270] x23: 886af900 x22: 88c13008 x21: 002003fb6e00 [ 35.806432] x20: 0040 x19: 002003f09008 x18: [ 35.813594] x17: x16: x15: 89ee3d48 [ 35.820756] x14: x13: x12: 0008 [ 35.827918] x11: db063d30 x10: 0960 x9 : 886afedc [ 35.835080] x8 : 00200186a900 x7 : x6 : [ 35.842242] x5 : 00200231d2b0 x4 : x3 : [ 35.849403] x2 : x1 : 0078 x0 : 0078 [ 35.856565] Call trace: [ 35.859013] drm_sched_entity_select_rq+0x314/0x380 [ 35.863906] drm_sched_main+0x1b0/0x49c [ 35.867752] kthread+0xe4/0xf0 [ 35.870814] ret_from_fork+0x10/0x20 [ 35.874401] Code: 8805fc24 35a5 17fffef9 f9800031 (885f7c22) [ 35.880513] ---[ end trace ]--- # glmark2-es2-drm === glmark2 2021.02 === OpenGL Information GL_VENDOR: etnaviv GL_RENDERER: Vivante GC7000 rev 6202 GL_VERSION:OpenGL ES 2.0 Mesa 22.1.0-devel Mesa is Lucas latest MR branch: lynxeye/etnaviv-gc7000-r6204. Reverting this patch on drm-next will make the oops go away. Any idea what's going wrong here? -michael
Re: [PATCH] i2c: at91: use dma safe buffers
Am 2022-04-05 15:58, schrieb codrin.ciubota...@microchip.com:
On 05.04.2022 14:09, Michael Walle wrote:
Am 2022-04-05 12:02, schrieb codrin.ciubota...@microchip.com:
On 05.04.2022 12:38, Michael Walle wrote:
Am 2022-04-05 11:23, schrieb codrin.ciubota...@microchip.com:
+ if (dev->use_dma) {
+ dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
If you want, you could just dev->buf = i2c_get_dma_safe...
But where is the error handling in that case? dev->buf will
be NULL, which is eventually passed to dma_map_single().
Also, I need the dma_buf for the i2c_put_dma_safe_msg_buf()
call anyway, because dev->buf will be modified during
processing.
You still:
if (!dev->buf) {
ret = -ENOMEM;
goto out;
}
So, at91_do_twi_transfer()/dma_map_single() will not be called.
Ahh, I misunderstood you. Yes, but as I said, I need the dma_buf
temporary variable anyway, because dev->buf is modified, eg. see
at91_twi_read_data_dma_callback().
at91_twi_read_data_dma_callback() is called as callback if
dma_async_issue_pending(dma->chan_rx) is called.
dma_async_issue_pending(dma->chan_rx) is called on
at91_twi_read_data_dma(), which is called in at91_do_twi_transfer(),
which we decided above to skip in case of error.
It is not about errors, you need the exact same pointer you
got from i2c_get_dma_safe_msg_buf() to be passed to
i2c_put_dma_safe_msg_buf(). And because (in some cases, it
isn't really obvious) the dev->buf will be advanced a few
bytes, I cannot pass dev->buf to i2c_put_dma_safe_msg_buf().
-michael
Re: [PATCH] i2c: at91: use dma safe buffers
Am 2022-04-05 12:02, schrieb codrin.ciubota...@microchip.com:
On 05.04.2022 12:38, Michael Walle wrote:
Am 2022-04-05 11:23, schrieb codrin.ciubota...@microchip.com:
+ if (dev->use_dma) {
+ dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
If you want, you could just dev->buf = i2c_get_dma_safe...
But where is the error handling in that case? dev->buf will
be NULL, which is eventually passed to dma_map_single().
Also, I need the dma_buf for the i2c_put_dma_safe_msg_buf()
call anyway, because dev->buf will be modified during
processing.
You still:
if (!dev->buf) {
ret = -ENOMEM;
goto out;
}
So, at91_do_twi_transfer()/dma_map_single() will not be called.
Ahh, I misunderstood you. Yes, but as I said, I need the dma_buf
temporary variable anyway, because dev->buf is modified, eg. see
at91_twi_read_data_dma_callback().
-michael
Re: [PATCH] i2c: at91: use dma safe buffers
Am 2022-04-05 11:23, schrieb codrin.ciubota...@microchip.com:
On 03.03.2022 18:17, Michael Walle wrote:
EXTERNAL EMAIL: Do not click links or open attachments unless you know
the content is safe
The supplied buffer might be on the stack and we get the following
error
message:
[3.312058] at91_i2c e0070600.i2c: rejecting DMA map of vmalloc
memory
Use i2c_{get,put}_dma_safe_msg_buf() to get a DMA-able memory region
if
necessary.
Cc: sta...@vger.kernel.org
Signed-off-by: Michael Walle
Reviewed-by: Codrin Ciubotariu
Thanks!
I'm not sure if or which Fixes: tag I should add to this patch. The
issue
seems to be since a very long time, but nobody seem to have triggered
it.
FWIW, I'm using the sff,sfp driver, which triggers this.
I think it should be:
Fixes: 60937b2cdbf9 ("i2c: at91: add dma support")
+ if (dev->use_dma) {
+ dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
If you want, you could just dev->buf = i2c_get_dma_safe...
But where is the error handling in that case? dev->buf will
be NULL, which is eventually passed to dma_map_single().
Also, I need the dma_buf for the i2c_put_dma_safe_msg_buf()
call anyway, because dev->buf will be modified during
processing.
-michael
Re: [PATCH] i2c: at91: use dma safe buffers
Hi all,
Am 2022-03-03 17:17, schrieb Michael Walle:
The supplied buffer might be on the stack and we get the following
error
message:
[3.312058] at91_i2c e0070600.i2c: rejecting DMA map of vmalloc
memory
Use i2c_{get,put}_dma_safe_msg_buf() to get a DMA-able memory region if
necessary.
Cc: sta...@vger.kernel.org
Signed-off-by: Michael Walle
Any news here?
---
I'm not sure if or which Fixes: tag I should add to this patch. The
issue
seems to be since a very long time, but nobody seem to have triggered
it.
FWIW, I'm using the sff,sfp driver, which triggers this.
-michael
[PATCH] i2c: at91: use dma safe buffers
The supplied buffer might be on the stack and we get the following error
message:
[3.312058] at91_i2c e0070600.i2c: rejecting DMA map of vmalloc memory
Use i2c_{get,put}_dma_safe_msg_buf() to get a DMA-able memory region if
necessary.
Cc: sta...@vger.kernel.org
Signed-off-by: Michael Walle
---
I'm not sure if or which Fixes: tag I should add to this patch. The issue
seems to be since a very long time, but nobody seem to have triggered it.
FWIW, I'm using the sff,sfp driver, which triggers this.
drivers/i2c/busses/i2c-at91-master.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/i2c/busses/i2c-at91-master.c
b/drivers/i2c/busses/i2c-at91-master.c
index b0eae94909f4..a7a22fedbaba 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -656,6 +656,7 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct
i2c_msg *msg, int num)
unsigned int_addr_flag = 0;
struct i2c_msg *m_start = msg;
bool is_read;
+ u8 *dma_buf;
dev_dbg(>dev, "at91_xfer: processing %d messages:\n", num);
@@ -703,7 +704,18 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct
i2c_msg *msg, int num)
dev->msg = m_start;
dev->recv_len_abort = false;
+ if (dev->use_dma) {
+ dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
+ if (!dma_buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ dev->buf = dma_buf;
+ }
+
+
ret = at91_do_twi_transfer(dev);
+ i2c_put_dma_safe_msg_buf(dma_buf, m_start, !ret);
ret = (ret < 0) ? ret : num;
out:
--
2.30.2
Re: [PATCH v2 0/3] drm/etnaviv: IOMMU related fixes
Am 2021-09-07 18:49, schrieb Michael Walle: This patch series fixes usage of the etnaviv driver with GPUs behind a IOMMU. It was tested on a NXP LS1028A SoC. Together with Lucas' MMU patches [1] there are not more (GPU internal) MMU nor (system) IOMMU faults on the LS1028A. [1] https://lists.freedesktop.org/archives/etnaviv/2021-August/003682.html changes since v1: - don't move the former dma_mask setup code around in patch 2/3. Will avoid any confusion. Michael Walle (3): drm/etnaviv: use PLATFORM_DEVID_NONE drm/etnaviv: fix dma configuration of the virtual device drm/etnaviv: use a 32 bit mask as coherent DMA mask drivers/gpu/drm/etnaviv/etnaviv_drv.c | 41 --- 1 file changed, 31 insertions(+), 10 deletions(-) ping? :) -michael
[PATCH v2 3/3] drm/etnaviv: use a 32 bit mask as coherent DMA mask
The STLB and the first command buffer (which is used to set up the TLBs)
has a 32 bit size restriction in hardware. There seems to be no way to
specify addresses larger than 32 bit. Keep it simple and restict the
addresses to the lower 4 GiB range for all coherent DMA memory
allocations.
Please note, that platform_device_alloc() will initialize dev->dma_mask
to point to pdev->platform_dma_mask, thus dma_set_mask() will work as
expected.
While at it, move the dma_mask setup code to the of_dma_configure() to
keep all the DMA setup code next to each other.
Suggested-by: Lucas Stach
Signed-off-by: Michael Walle
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 54eb653ca295..0b756ecb1bc2 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -613,6 +613,24 @@ static int etnaviv_pdev_probe(struct platform_device *pdev)
component_match_add(dev, , compare_str, names[i]);
}
+ /*
+* PTA and MTLB can have 40 bit base addresses, but
+* unfortunately, an entry in the MTLB can only point to a
+* 32 bit base address of a STLB. Moreover, to initialize the
+* MMU we need a command buffer with a 32 bit address because
+* without an MMU there is only an indentity mapping between
+* the internal 32 bit addresses and the bus addresses.
+*
+* To make things easy, we set the dma_coherent_mask to 32
+* bit to make sure we are allocating the command buffers and
+* TLBs in the lower 4 GiB address space.
+*/
+ if (dma_set_mask(>dev, DMA_BIT_MASK(40)) ||
+ dma_set_coherent_mask(>dev, DMA_BIT_MASK(32))) {
+ dev_dbg(>dev, "No suitable DMA available\n");
+ return -ENODEV;
+ }
+
/*
* Apply the same DMA configuration to the virtual etnaviv
* device as the GPU we found. This assumes that all Vivante
@@ -671,8 +689,6 @@ static int __init etnaviv_init(void)
of_node_put(np);
goto unregister_platform_driver;
}
- pdev->dev.coherent_dma_mask = DMA_BIT_MASK(40);
- pdev->dev.dma_mask = >dev.coherent_dma_mask;
ret = platform_device_add(pdev);
if (ret) {
--
2.30.2
