* [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR
@ 2022-01-13 14:43 Neil Armstrong
2022-01-17 9:53 ` Robert Foss
2022-07-31 20:07 ` Dmitry Osipenko
0 siblings, 2 replies; 11+ messages in thread
From: Neil Armstrong @ 2022-01-13 14:43 UTC (permalink / raw)
To: andrzej.hajda, robert.foss
Cc: Laurent.pinchart, jonas, jernej.skrabec, dri-devel, linux-kernel,
Neil Armstrong
This adds support for DRM_BRIDGE_ATTACH_NO_CONNECTOR by adding the
bridge get_edid() and detect() callbacks after refactoring the connector
get_modes() and connector_detect() callbacks.
In order to keep the bridge working, extra code in get_modes() has been
moved to more logical places.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
drivers/gpu/drm/bridge/sii902x.c | 129 ++++++++++++++++++++++++-------
1 file changed, 99 insertions(+), 30 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 89558e581530..65549fbfdc87 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -166,10 +166,12 @@ struct sii902x {
struct i2c_client *i2c;
struct regmap *regmap;
struct drm_bridge bridge;
+ struct drm_bridge *next_bridge;
struct drm_connector connector;
struct gpio_desc *reset_gpio;
struct i2c_mux_core *i2cmux;
struct regulator_bulk_data supplies[2];
+ bool sink_is_hdmi;
/*
* Mutex protects audio and video functions from interfering
* each other, by keeping their i2c command sequences atomic.
@@ -245,10 +247,8 @@ static void sii902x_reset(struct sii902x *sii902x)
gpiod_set_value(sii902x->reset_gpio, 0);
}
-static enum drm_connector_status
-sii902x_connector_detect(struct drm_connector *connector, bool force)
+static enum drm_connector_status sii902x_detect(struct sii902x *sii902x)
{
- struct sii902x *sii902x = connector_to_sii902x(connector);
unsigned int status;
mutex_lock(&sii902x->mutex);
@@ -261,6 +261,14 @@ sii902x_connector_detect(struct drm_connector *connector, bool force)
connector_status_connected : connector_status_disconnected;
}
+static enum drm_connector_status
+sii902x_connector_detect(struct drm_connector *connector, bool force)
+{
+ struct sii902x *sii902x = connector_to_sii902x(connector);
+
+ return sii902x_detect(sii902x);
+}
+
static const struct drm_connector_funcs sii902x_connector_funcs = {
.detect = sii902x_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
@@ -270,42 +278,40 @@ static const struct drm_connector_funcs sii902x_connector_funcs = {
.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
};
-static int sii902x_get_modes(struct drm_connector *connector)
+static struct edid *sii902x_get_edid(struct sii902x *sii902x,
+ struct drm_connector *connector)
{
- struct sii902x *sii902x = connector_to_sii902x(connector);
- u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
- u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
struct edid *edid;
- int num = 0, ret;
mutex_lock(&sii902x->mutex);
edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
- drm_connector_update_edid_property(connector, edid);
if (edid) {
if (drm_detect_hdmi_monitor(edid))
- output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
-
- num = drm_add_edid_modes(connector, edid);
- kfree(edid);
+ sii902x->sink_is_hdmi = true;
+ else
+ sii902x->sink_is_hdmi = false;
}
- ret = drm_display_info_set_bus_formats(&connector->display_info,
- &bus_format, 1);
- if (ret)
- goto error_out;
+ mutex_unlock(&sii902x->mutex);
- ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
- SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
- if (ret)
- goto error_out;
+ return edid;
+}
- ret = num;
+static int sii902x_get_modes(struct drm_connector *connector)
+{
+ struct sii902x *sii902x = connector_to_sii902x(connector);
+ struct edid *edid;
+ int num = 0;
-error_out:
- mutex_unlock(&sii902x->mutex);
+ edid = sii902x_get_edid(sii902x, connector);
+ drm_connector_update_edid_property(connector, edid);
+ if (edid) {
+ num = drm_add_edid_modes(connector, edid);
+ kfree(edid);
+ }
- return ret;
+ return num;
}
static enum drm_mode_status sii902x_mode_valid(struct drm_connector *connector,
@@ -354,12 +360,16 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
const struct drm_display_mode *adj)
{
struct sii902x *sii902x = bridge_to_sii902x(bridge);
+ u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
struct regmap *regmap = sii902x->regmap;
u8 buf[HDMI_INFOFRAME_SIZE(AVI)];
struct hdmi_avi_infoframe frame;
u16 pixel_clock_10kHz = adj->clock / 10;
int ret;
+ if (sii902x->sink_is_hdmi)
+ output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
+
buf[0] = pixel_clock_10kHz & 0xff;
buf[1] = pixel_clock_10kHz >> 8;
buf[2] = drm_mode_vrefresh(adj);
@@ -375,6 +385,11 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
mutex_lock(&sii902x->mutex);
+ ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
+ SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
+ if (ret)
+ goto out;
+
ret = regmap_bulk_write(regmap, SII902X_TPI_VIDEO_DATA, buf, 10);
if (ret)
goto out;
@@ -405,13 +420,13 @@ static int sii902x_bridge_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)
{
struct sii902x *sii902x = bridge_to_sii902x(bridge);
+ u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
struct drm_device *drm = bridge->dev;
int ret;
- if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
- DRM_ERROR("Fix bridge driver to make connector optional!");
- return -EINVAL;
- }
+ if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
+ return drm_bridge_attach(bridge->encoder, sii902x->next_bridge,
+ bridge, flags);
drm_connector_helper_add(&sii902x->connector,
&sii902x_connector_helper_funcs);
@@ -433,16 +448,38 @@ static int sii902x_bridge_attach(struct drm_bridge *bridge,
else
sii902x->connector.polled = DRM_CONNECTOR_POLL_CONNECT;
+ ret = drm_display_info_set_bus_formats(&sii902x->connector.display_info,
+ &bus_format, 1);
+ if (ret)
+ return ret;
+
drm_connector_attach_encoder(&sii902x->connector, bridge->encoder);
return 0;
}
+static enum drm_connector_status sii902x_bridge_detect(struct drm_bridge *bridge)
+{
+ struct sii902x *sii902x = bridge_to_sii902x(bridge);
+
+ return sii902x_detect(sii902x);
+}
+
+static struct edid *sii902x_bridge_get_edid(struct drm_bridge *bridge,
+ struct drm_connector *connector)
+{
+ struct sii902x *sii902x = bridge_to_sii902x(bridge);
+
+ return sii902x_get_edid(sii902x, connector);
+}
+
static const struct drm_bridge_funcs sii902x_bridge_funcs = {
.attach = sii902x_bridge_attach,
.mode_set = sii902x_bridge_mode_set,
.disable = sii902x_bridge_disable,
.enable = sii902x_bridge_enable,
+ .detect = sii902x_bridge_detect,
+ .get_edid = sii902x_bridge_get_edid,
};
static int sii902x_mute(struct sii902x *sii902x, bool mute)
@@ -829,8 +866,12 @@ static irqreturn_t sii902x_interrupt(int irq, void *data)
mutex_unlock(&sii902x->mutex);
- if ((status & SII902X_HOTPLUG_EVENT) && sii902x->bridge.dev)
+ if ((status & SII902X_HOTPLUG_EVENT) && sii902x->bridge.dev) {
drm_helper_hpd_irq_event(sii902x->bridge.dev);
+ drm_bridge_hpd_notify(&sii902x->bridge, (status & SII902X_PLUGGED_STATUS)
+ ? connector_status_connected
+ : connector_status_disconnected);
+ }
return IRQ_HANDLED;
}
@@ -1001,6 +1042,11 @@ static int sii902x_init(struct sii902x *sii902x)
sii902x->bridge.funcs = &sii902x_bridge_funcs;
sii902x->bridge.of_node = dev->of_node;
sii902x->bridge.timings = &default_sii902x_timings;
+ sii902x->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
+
+ if (sii902x->i2c->irq > 0)
+ sii902x->bridge.ops |= DRM_BRIDGE_OP_HPD;
+
drm_bridge_add(&sii902x->bridge);
sii902x_audio_codec_init(sii902x, dev);
@@ -1022,6 +1068,7 @@ static int sii902x_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
struct device *dev = &client->dev;
+ struct device_node *endpoint;
struct sii902x *sii902x;
int ret;
@@ -1049,6 +1096,28 @@ static int sii902x_probe(struct i2c_client *client,
return PTR_ERR(sii902x->reset_gpio);
}
+ endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
+ if (endpoint) {
+ struct device_node *remote = of_graph_get_remote_port_parent(endpoint);
+
+ of_node_put(endpoint);
+ if (!remote) {
+ dev_err(dev, "Endpoint in port@1 unconnected\n");
+ return -ENODEV;
+ }
+
+ if (!of_device_is_available(remote)) {
+ dev_err(dev, "port@1 remote device is disabled\n");
+ of_node_put(remote);
+ return -ENODEV;
+ }
+
+ sii902x->next_bridge = of_drm_find_bridge(remote);
+ of_node_put(remote);
+ if (!sii902x->next_bridge)
+ return -EPROBE_DEFER;
+ }
+
mutex_init(&sii902x->mutex);
sii902x->supplies[0].supply = "iovcc";
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR
2022-01-13 14:43 [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR Neil Armstrong
@ 2022-01-17 9:53 ` Robert Foss
2022-07-31 20:07 ` Dmitry Osipenko
1 sibling, 0 replies; 11+ messages in thread
From: Robert Foss @ 2022-01-17 9:53 UTC (permalink / raw)
To: Neil Armstrong
Cc: andrzej.hajda, laurent.pinchart, jonas, jernej.skrabec,
dri-devel, linux-kernel
Hey Neil,
On Thu, 13 Jan 2022 at 15:43, Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This adds support for DRM_BRIDGE_ATTACH_NO_CONNECTOR by adding the
> bridge get_edid() and detect() callbacks after refactoring the connector
> get_modes() and connector_detect() callbacks.
>
> In order to keep the bridge working, extra code in get_modes() has been
> moved to more logical places.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> drivers/gpu/drm/bridge/sii902x.c | 129 ++++++++++++++++++++++++-------
> 1 file changed, 99 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index 89558e581530..65549fbfdc87 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -166,10 +166,12 @@ struct sii902x {
> struct i2c_client *i2c;
> struct regmap *regmap;
> struct drm_bridge bridge;
> + struct drm_bridge *next_bridge;
> struct drm_connector connector;
> struct gpio_desc *reset_gpio;
> struct i2c_mux_core *i2cmux;
> struct regulator_bulk_data supplies[2];
> + bool sink_is_hdmi;
> /*
> * Mutex protects audio and video functions from interfering
> * each other, by keeping their i2c command sequences atomic.
> @@ -245,10 +247,8 @@ static void sii902x_reset(struct sii902x *sii902x)
> gpiod_set_value(sii902x->reset_gpio, 0);
> }
>
> -static enum drm_connector_status
> -sii902x_connector_detect(struct drm_connector *connector, bool force)
> +static enum drm_connector_status sii902x_detect(struct sii902x *sii902x)
> {
> - struct sii902x *sii902x = connector_to_sii902x(connector);
> unsigned int status;
>
> mutex_lock(&sii902x->mutex);
> @@ -261,6 +261,14 @@ sii902x_connector_detect(struct drm_connector *connector, bool force)
> connector_status_connected : connector_status_disconnected;
> }
>
> +static enum drm_connector_status
> +sii902x_connector_detect(struct drm_connector *connector, bool force)
> +{
> + struct sii902x *sii902x = connector_to_sii902x(connector);
> +
> + return sii902x_detect(sii902x);
> +}
> +
> static const struct drm_connector_funcs sii902x_connector_funcs = {
> .detect = sii902x_connector_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
> @@ -270,42 +278,40 @@ static const struct drm_connector_funcs sii902x_connector_funcs = {
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> };
>
> -static int sii902x_get_modes(struct drm_connector *connector)
> +static struct edid *sii902x_get_edid(struct sii902x *sii902x,
> + struct drm_connector *connector)
> {
> - struct sii902x *sii902x = connector_to_sii902x(connector);
> - u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> - u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
> struct edid *edid;
> - int num = 0, ret;
>
> mutex_lock(&sii902x->mutex);
>
> edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
> - drm_connector_update_edid_property(connector, edid);
> if (edid) {
> if (drm_detect_hdmi_monitor(edid))
> - output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
> -
> - num = drm_add_edid_modes(connector, edid);
> - kfree(edid);
> + sii902x->sink_is_hdmi = true;
> + else
> + sii902x->sink_is_hdmi = false;
> }
>
> - ret = drm_display_info_set_bus_formats(&connector->display_info,
> - &bus_format, 1);
> - if (ret)
> - goto error_out;
> + mutex_unlock(&sii902x->mutex);
>
> - ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
> - SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
> - if (ret)
> - goto error_out;
> + return edid;
> +}
>
> - ret = num;
> +static int sii902x_get_modes(struct drm_connector *connector)
> +{
> + struct sii902x *sii902x = connector_to_sii902x(connector);
> + struct edid *edid;
> + int num = 0;
>
> -error_out:
> - mutex_unlock(&sii902x->mutex);
> + edid = sii902x_get_edid(sii902x, connector);
> + drm_connector_update_edid_property(connector, edid);
> + if (edid) {
> + num = drm_add_edid_modes(connector, edid);
> + kfree(edid);
> + }
>
> - return ret;
> + return num;
> }
>
> static enum drm_mode_status sii902x_mode_valid(struct drm_connector *connector,
> @@ -354,12 +360,16 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
> const struct drm_display_mode *adj)
> {
> struct sii902x *sii902x = bridge_to_sii902x(bridge);
> + u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
> struct regmap *regmap = sii902x->regmap;
> u8 buf[HDMI_INFOFRAME_SIZE(AVI)];
> struct hdmi_avi_infoframe frame;
> u16 pixel_clock_10kHz = adj->clock / 10;
> int ret;
>
> + if (sii902x->sink_is_hdmi)
> + output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
> +
> buf[0] = pixel_clock_10kHz & 0xff;
> buf[1] = pixel_clock_10kHz >> 8;
> buf[2] = drm_mode_vrefresh(adj);
> @@ -375,6 +385,11 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
>
> mutex_lock(&sii902x->mutex);
>
> + ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
> + SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
> + if (ret)
> + goto out;
> +
> ret = regmap_bulk_write(regmap, SII902X_TPI_VIDEO_DATA, buf, 10);
> if (ret)
> goto out;
> @@ -405,13 +420,13 @@ static int sii902x_bridge_attach(struct drm_bridge *bridge,
> enum drm_bridge_attach_flags flags)
> {
> struct sii902x *sii902x = bridge_to_sii902x(bridge);
> + u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> struct drm_device *drm = bridge->dev;
> int ret;
>
> - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> - DRM_ERROR("Fix bridge driver to make connector optional!");
> - return -EINVAL;
> - }
> + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
> + return drm_bridge_attach(bridge->encoder, sii902x->next_bridge,
> + bridge, flags);
>
> drm_connector_helper_add(&sii902x->connector,
> &sii902x_connector_helper_funcs);
> @@ -433,16 +448,38 @@ static int sii902x_bridge_attach(struct drm_bridge *bridge,
> else
> sii902x->connector.polled = DRM_CONNECTOR_POLL_CONNECT;
>
> + ret = drm_display_info_set_bus_formats(&sii902x->connector.display_info,
> + &bus_format, 1);
> + if (ret)
> + return ret;
> +
> drm_connector_attach_encoder(&sii902x->connector, bridge->encoder);
>
> return 0;
> }
>
> +static enum drm_connector_status sii902x_bridge_detect(struct drm_bridge *bridge)
> +{
> + struct sii902x *sii902x = bridge_to_sii902x(bridge);
> +
> + return sii902x_detect(sii902x);
> +}
> +
> +static struct edid *sii902x_bridge_get_edid(struct drm_bridge *bridge,
> + struct drm_connector *connector)
> +{
> + struct sii902x *sii902x = bridge_to_sii902x(bridge);
> +
> + return sii902x_get_edid(sii902x, connector);
> +}
> +
> static const struct drm_bridge_funcs sii902x_bridge_funcs = {
> .attach = sii902x_bridge_attach,
> .mode_set = sii902x_bridge_mode_set,
> .disable = sii902x_bridge_disable,
> .enable = sii902x_bridge_enable,
> + .detect = sii902x_bridge_detect,
> + .get_edid = sii902x_bridge_get_edid,
> };
>
> static int sii902x_mute(struct sii902x *sii902x, bool mute)
> @@ -829,8 +866,12 @@ static irqreturn_t sii902x_interrupt(int irq, void *data)
>
> mutex_unlock(&sii902x->mutex);
>
> - if ((status & SII902X_HOTPLUG_EVENT) && sii902x->bridge.dev)
> + if ((status & SII902X_HOTPLUG_EVENT) && sii902x->bridge.dev) {
> drm_helper_hpd_irq_event(sii902x->bridge.dev);
> + drm_bridge_hpd_notify(&sii902x->bridge, (status & SII902X_PLUGGED_STATUS)
> + ? connector_status_connected
> + : connector_status_disconnected);
> + }
>
> return IRQ_HANDLED;
> }
> @@ -1001,6 +1042,11 @@ static int sii902x_init(struct sii902x *sii902x)
> sii902x->bridge.funcs = &sii902x_bridge_funcs;
> sii902x->bridge.of_node = dev->of_node;
> sii902x->bridge.timings = &default_sii902x_timings;
> + sii902x->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
> +
> + if (sii902x->i2c->irq > 0)
> + sii902x->bridge.ops |= DRM_BRIDGE_OP_HPD;
> +
> drm_bridge_add(&sii902x->bridge);
>
> sii902x_audio_codec_init(sii902x, dev);
> @@ -1022,6 +1068,7 @@ static int sii902x_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> struct device *dev = &client->dev;
> + struct device_node *endpoint;
> struct sii902x *sii902x;
> int ret;
>
> @@ -1049,6 +1096,28 @@ static int sii902x_probe(struct i2c_client *client,
> return PTR_ERR(sii902x->reset_gpio);
> }
>
> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> + if (endpoint) {
> + struct device_node *remote = of_graph_get_remote_port_parent(endpoint);
> +
> + of_node_put(endpoint);
> + if (!remote) {
> + dev_err(dev, "Endpoint in port@1 unconnected\n");
> + return -ENODEV;
> + }
> +
> + if (!of_device_is_available(remote)) {
> + dev_err(dev, "port@1 remote device is disabled\n");
> + of_node_put(remote);
> + return -ENODEV;
> + }
> +
> + sii902x->next_bridge = of_drm_find_bridge(remote);
> + of_node_put(remote);
> + if (!sii902x->next_bridge)
> + return -EPROBE_DEFER;
> + }
> +
> mutex_init(&sii902x->mutex);
>
> sii902x->supplies[0].supply = "iovcc";
Reviewed-by: Robert Foss <robert.foss@linaro.org>
Applied to drm-misc-next.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR
2022-01-13 14:43 [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR Neil Armstrong
2022-01-17 9:53 ` Robert Foss
@ 2022-07-31 20:07 ` Dmitry Osipenko
2022-08-08 9:15 ` Neil Armstrong
1 sibling, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2022-07-31 20:07 UTC (permalink / raw)
To: Neil Armstrong, andrzej.hajda, robert.foss
Cc: jernej.skrabec, jonas, linux-kernel, dri-devel, Laurent.pinchart
13.01.2022 17:43, Neil Armstrong пишет:
> This adds support for DRM_BRIDGE_ATTACH_NO_CONNECTOR by adding the
> bridge get_edid() and detect() callbacks after refactoring the connector
> get_modes() and connector_detect() callbacks.
>
> In order to keep the bridge working, extra code in get_modes() has been
> moved to more logical places.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
> drivers/gpu/drm/bridge/sii902x.c | 129 ++++++++++++++++++++++++-------
> 1 file changed, 99 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index 89558e581530..65549fbfdc87 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -166,10 +166,12 @@ struct sii902x {
> struct i2c_client *i2c;
> struct regmap *regmap;
> struct drm_bridge bridge;
> + struct drm_bridge *next_bridge;
> struct drm_connector connector;
> struct gpio_desc *reset_gpio;
> struct i2c_mux_core *i2cmux;
> struct regulator_bulk_data supplies[2];
> + bool sink_is_hdmi;
> /*
> * Mutex protects audio and video functions from interfering
> * each other, by keeping their i2c command sequences atomic.
> @@ -245,10 +247,8 @@ static void sii902x_reset(struct sii902x *sii902x)
> gpiod_set_value(sii902x->reset_gpio, 0);
> }
>
> -static enum drm_connector_status
> -sii902x_connector_detect(struct drm_connector *connector, bool force)
> +static enum drm_connector_status sii902x_detect(struct sii902x *sii902x)
> {
> - struct sii902x *sii902x = connector_to_sii902x(connector);
> unsigned int status;
>
> mutex_lock(&sii902x->mutex);
> @@ -261,6 +261,14 @@ sii902x_connector_detect(struct drm_connector *connector, bool force)
> connector_status_connected : connector_status_disconnected;
> }
>
> +static enum drm_connector_status
> +sii902x_connector_detect(struct drm_connector *connector, bool force)
> +{
> + struct sii902x *sii902x = connector_to_sii902x(connector);
> +
> + return sii902x_detect(sii902x);
> +}
> +
> static const struct drm_connector_funcs sii902x_connector_funcs = {
> .detect = sii902x_connector_detect,
> .fill_modes = drm_helper_probe_single_connector_modes,
> @@ -270,42 +278,40 @@ static const struct drm_connector_funcs sii902x_connector_funcs = {
> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> };
>
> -static int sii902x_get_modes(struct drm_connector *connector)
> +static struct edid *sii902x_get_edid(struct sii902x *sii902x,
> + struct drm_connector *connector)
> {
> - struct sii902x *sii902x = connector_to_sii902x(connector);
> - u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> - u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
> struct edid *edid;
> - int num = 0, ret;
>
> mutex_lock(&sii902x->mutex);
>
> edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
> - drm_connector_update_edid_property(connector, edid);
> if (edid) {
> if (drm_detect_hdmi_monitor(edid))
> - output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
> -
> - num = drm_add_edid_modes(connector, edid);
> - kfree(edid);
> + sii902x->sink_is_hdmi = true;
> + else
> + sii902x->sink_is_hdmi = false;
> }
>
> - ret = drm_display_info_set_bus_formats(&connector->display_info,
> - &bus_format, 1);
> - if (ret)
> - goto error_out;
> + mutex_unlock(&sii902x->mutex);
>
> - ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
> - SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
> - if (ret)
> - goto error_out;
> + return edid;
> +}
>
> - ret = num;
> +static int sii902x_get_modes(struct drm_connector *connector)
> +{
> + struct sii902x *sii902x = connector_to_sii902x(connector);
> + struct edid *edid;
> + int num = 0;
>
> -error_out:
> - mutex_unlock(&sii902x->mutex);
> + edid = sii902x_get_edid(sii902x, connector);
> + drm_connector_update_edid_property(connector, edid);
> + if (edid) {
> + num = drm_add_edid_modes(connector, edid);
> + kfree(edid);
> + }
>
> - return ret;
> + return num;
> }
>
> static enum drm_mode_status sii902x_mode_valid(struct drm_connector *connector,
> @@ -354,12 +360,16 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
> const struct drm_display_mode *adj)
> {
> struct sii902x *sii902x = bridge_to_sii902x(bridge);
> + u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
> struct regmap *regmap = sii902x->regmap;
> u8 buf[HDMI_INFOFRAME_SIZE(AVI)];
> struct hdmi_avi_infoframe frame;
> u16 pixel_clock_10kHz = adj->clock / 10;
> int ret;
>
> + if (sii902x->sink_is_hdmi)
> + output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
> +
> buf[0] = pixel_clock_10kHz & 0xff;
> buf[1] = pixel_clock_10kHz >> 8;
> buf[2] = drm_mode_vrefresh(adj);
> @@ -375,6 +385,11 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
>
> mutex_lock(&sii902x->mutex);
>
> + ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
> + SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
> + if (ret)
> + goto out;
> +
> ret = regmap_bulk_write(regmap, SII902X_TPI_VIDEO_DATA, buf, 10);
> if (ret)
> goto out;
> @@ -405,13 +420,13 @@ static int sii902x_bridge_attach(struct drm_bridge *bridge,
> enum drm_bridge_attach_flags flags)
> {
> struct sii902x *sii902x = bridge_to_sii902x(bridge);
> + u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> struct drm_device *drm = bridge->dev;
> int ret;
>
> - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> - DRM_ERROR("Fix bridge driver to make connector optional!");
> - return -EINVAL;
> - }
> + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
> + return drm_bridge_attach(bridge->encoder, sii902x->next_bridge,
> + bridge, flags);
>
> drm_connector_helper_add(&sii902x->connector,
> &sii902x_connector_helper_funcs);
> @@ -433,16 +448,38 @@ static int sii902x_bridge_attach(struct drm_bridge *bridge,
> else
> sii902x->connector.polled = DRM_CONNECTOR_POLL_CONNECT;
>
> + ret = drm_display_info_set_bus_formats(&sii902x->connector.display_info,
> + &bus_format, 1);
> + if (ret)
> + return ret;
> +
> drm_connector_attach_encoder(&sii902x->connector, bridge->encoder);
>
> return 0;
> }
>
> +static enum drm_connector_status sii902x_bridge_detect(struct drm_bridge *bridge)
> +{
> + struct sii902x *sii902x = bridge_to_sii902x(bridge);
> +
> + return sii902x_detect(sii902x);
> +}
> +
> +static struct edid *sii902x_bridge_get_edid(struct drm_bridge *bridge,
> + struct drm_connector *connector)
> +{
> + struct sii902x *sii902x = bridge_to_sii902x(bridge);
> +
> + return sii902x_get_edid(sii902x, connector);
> +}
> +
> static const struct drm_bridge_funcs sii902x_bridge_funcs = {
> .attach = sii902x_bridge_attach,
> .mode_set = sii902x_bridge_mode_set,
> .disable = sii902x_bridge_disable,
> .enable = sii902x_bridge_enable,
> + .detect = sii902x_bridge_detect,
> + .get_edid = sii902x_bridge_get_edid,
> };
>
> static int sii902x_mute(struct sii902x *sii902x, bool mute)
> @@ -829,8 +866,12 @@ static irqreturn_t sii902x_interrupt(int irq, void *data)
>
> mutex_unlock(&sii902x->mutex);
>
> - if ((status & SII902X_HOTPLUG_EVENT) && sii902x->bridge.dev)
> + if ((status & SII902X_HOTPLUG_EVENT) && sii902x->bridge.dev) {
> drm_helper_hpd_irq_event(sii902x->bridge.dev);
> + drm_bridge_hpd_notify(&sii902x->bridge, (status & SII902X_PLUGGED_STATUS)
> + ? connector_status_connected
> + : connector_status_disconnected);
> + }
>
> return IRQ_HANDLED;
> }
> @@ -1001,6 +1042,11 @@ static int sii902x_init(struct sii902x *sii902x)
> sii902x->bridge.funcs = &sii902x_bridge_funcs;
> sii902x->bridge.of_node = dev->of_node;
> sii902x->bridge.timings = &default_sii902x_timings;
> + sii902x->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
> +
> + if (sii902x->i2c->irq > 0)
> + sii902x->bridge.ops |= DRM_BRIDGE_OP_HPD;
> +
> drm_bridge_add(&sii902x->bridge);
>
> sii902x_audio_codec_init(sii902x, dev);
> @@ -1022,6 +1068,7 @@ static int sii902x_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> struct device *dev = &client->dev;
> + struct device_node *endpoint;
> struct sii902x *sii902x;
> int ret;
>
> @@ -1049,6 +1096,28 @@ static int sii902x_probe(struct i2c_client *client,
> return PTR_ERR(sii902x->reset_gpio);
> }
>
> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> + if (endpoint) {
> + struct device_node *remote = of_graph_get_remote_port_parent(endpoint);
> +
> + of_node_put(endpoint);
> + if (!remote) {
> + dev_err(dev, "Endpoint in port@1 unconnected\n");
> + return -ENODEV;
> + }
> +
> + if (!of_device_is_available(remote)) {
> + dev_err(dev, "port@1 remote device is disabled\n");
> + of_node_put(remote);
> + return -ENODEV;
> + }
> +
> + sii902x->next_bridge = of_drm_find_bridge(remote);
> + of_node_put(remote);
> + if (!sii902x->next_bridge)
> + return -EPROBE_DEFER;
Hi,
This patch broke ARM/QEMU vexpress display because of_drm_find_bridge()
always fail with -EPROBE_DEFER. Reverting this patch returns display
back. Please fix or revert, thanks in advance.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR
2022-07-31 20:07 ` Dmitry Osipenko
@ 2022-08-08 9:15 ` Neil Armstrong
2022-08-08 9:51 ` Neil Armstrong
2022-08-15 0:18 ` Dmitry Osipenko
0 siblings, 2 replies; 11+ messages in thread
From: Neil Armstrong @ 2022-08-08 9:15 UTC (permalink / raw)
To: Dmitry Osipenko, andrzej.hajda, robert.foss
Cc: jernej.skrabec, jonas, linux-kernel, dri-devel, Laurent.pinchart
Hi Dmitry,
On 31/07/2022 22:07, Dmitry Osipenko wrote:
> 13.01.2022 17:43, Neil Armstrong пишет:
>> This adds support for DRM_BRIDGE_ATTACH_NO_CONNECTOR by adding the
>> bridge get_edid() and detect() callbacks after refactoring the connector
>> get_modes() and connector_detect() callbacks.
>>
>> In order to keep the bridge working, extra code in get_modes() has been
>> moved to more logical places.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> ---
>> drivers/gpu/drm/bridge/sii902x.c | 129 ++++++++++++++++++++++++-------
>>
1 file changed, 99 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
>> index 89558e581530..65549fbfdc87 100644
>> --- a/drivers/gpu/drm/bridge/sii902x.c
>> +++ b/drivers/gpu/drm/bridge/sii902x.c
>> @@ -166,10 +166,12 @@ struct sii902x {
>> struct i2c_client *i2c;
>> struct regmap *regmap;
>> struct drm_bridge bridge;
>> + struct drm_bridge *next_bridge;
>> struct drm_connector connector;
>> struct gpio_desc *reset_gpio;
>> struct i2c_mux_core *i2cmux;
>> struct regulator_bulk_data supplies[2];
>> + bool sink_is_hdmi;
>> /*
>> * Mutex protects audio and video functions from interfering
>> * each other, by keeping their i2c command sequences atomic.
>> @@ -245,10 +247,8 @@ static void sii902x_reset(struct sii902x *sii902x)
>> gpiod_set_value(sii902x->reset_gpio, 0);
>> }
>>
>> -static enum drm_connector_status
>> -sii902x_connector_detect(struct drm_connector *connector, bool force)
>> +static enum drm_connector_status sii902x_detect(struct sii902x *sii902x)
>> {
>> - struct sii902x *sii902x = connector_to_sii902x(connector);
>> unsigned int status;
>>
>> mutex_lock(&sii902x->mutex);
>> @@ -261,6 +261,14 @@ sii902x_connector_detect(struct drm_connector *connector, bool force)
>> connector_status_connected : connector_status_disconnected;
>> }
>>
>> +static enum drm_connector_status
>> +sii902x_connector_detect(struct drm_connector *connector, bool force)
>> +{
>> + struct sii902x *sii902x = connector_to_sii902x(connector);
>> +
>> + return sii902x_detect(sii902x);
>> +}
>> +
>> static const struct drm_connector_funcs sii902x_connector_funcs = {
>> .detect = sii902x_connector_detect,
>> .fill_modes = drm_helper_probe_single_connector_modes,
>> @@ -270,42 +278,40 @@ static const struct drm_connector_funcs sii902x_connector_funcs = {
>> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> };
>>
>> -static int sii902x_get_modes(struct drm_connector *connector)
>> +static struct edid *sii902x_get_edid(struct sii902x *sii902x,
>> + struct drm_connector *connector)
>> {
>> - struct sii902x *sii902x = connector_to_sii902x(connector);
>> - u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
>> - u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
>> struct edid *edid;
>> - int num = 0, ret;
>>
>> mutex_lock(&sii902x->mutex);
>>
>> edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
>> - drm_connector_update_edid_property(connector, edid);
>> if (edid) {
>> if (drm_detect_hdmi_monitor(edid))
>> - output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
>> -
>> - num = drm_add_edid_modes(connector, edid);
>> - kfree(edid);
>> + sii902x->sink_is_hdmi = true;
>> + else
>> + sii902x->sink_is_hdmi = false;
>> }
>>
>> - ret = drm_display_info_set_bus_formats(&connector->display_info,
>> - &bus_format, 1);
>> - if (ret)
>> - goto error_out;
>> + mutex_unlock(&sii902x->mutex);
>>
>> - ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
>> - SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
>> - if (ret)
>> - goto error_out;
>> + return edid;
>> +}
>>
>> - ret = num;
>> +static int sii902x_get_modes(struct drm_connector *connector)
>> +{
>> + struct sii902x *sii902x = connector_to_sii902x(connector);
>> + struct edid *edid;
>> + int num = 0;
>>
>> -error_out:
>> - mutex_unlock(&sii902x->mutex);
>> + edid = sii902x_get_edid(sii902x, connector);
>> + drm_connector_update_edid_property(connector, edid);
>> + if (edid) {
>> + num = drm_add_edid_modes(connector, edid);
>> + kfree(edid);
>> + }
>>
>> - return ret;
>> + return num;
>> }
>>
>> static enum drm_mode_status sii902x_mode_valid(struct drm_connector *connector,
>> @@ -354,12 +360,16 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
>> const struct drm_display_mode *adj)
>> {
>> struct sii902x *sii902x = bridge_to_sii902x(bridge);
>> + u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
>> struct regmap *regmap = sii902x->regmap;
>> u8 buf[HDMI_INFOFRAME_SIZE(AVI)];
>> struct hdmi_avi_infoframe frame;
>> u16 pixel_clock_10kHz = adj->clock / 10;
>> int ret;
>>
>> + if (sii902x->sink_is_hdmi)
>> + output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
>> +
>> buf[0] = pixel_clock_10kHz & 0xff;
>> buf[1] = pixel_clock_10kHz >> 8;
>> buf[2] = drm_mode_vrefresh(adj);
>> @@ -375,6 +385,11 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
>>
>> mutex_lock(&sii902x->mutex);
>>
>> + ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
>> + SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
>> + if (ret)
>> + goto out;
>> +
>> ret = regmap_bulk_write(regmap, SII902X_TPI_VIDEO_DATA, buf, 10);
>> if (ret)
>> goto out;
>> @@ -405,13 +420,13 @@ static int sii902x_bridge_attach(struct drm_bridge *bridge,
>> enum drm_bridge_attach_flags flags)
>> {
>> struct sii902x *sii902x = bridge_to_sii902x(bridge);
>> + u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
>> struct drm_device *drm = bridge->dev;
>> int ret;
>>
>> - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
>> - DRM_ERROR("Fix bridge driver to make connector optional!");
>> - return -EINVAL;
>> - }
>> + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
>> + return drm_bridge_attach(bridge->encoder, sii902x->next_bridge,
>> + bridge, flags);
>>
>> drm_connector_helper_add(&sii902x->connector,
>> &sii902x_connector_helper_funcs);
>> @@ -433,16 +448,38 @@ static int sii902x_bridge_attach(struct drm_bridge *bridge,
>> else
>> sii902x->connector.polled = DRM_CONNECTOR_POLL_CONNECT;
>>
>> + ret = drm_display_info_set_bus_formats(&sii902x->connector.display_info,
>> + &bus_format, 1);
>> + if (ret)
>> + return ret;
>> +
>> drm_connector_attach_encoder(&sii902x->connector, bridge->encoder);
>>
>> return 0;
>> }
>>
>> +static enum drm_connector_status sii902x_bridge_detect(struct drm_bridge *bridge)
>> +{
>> + struct sii902x *sii902x = bridge_to_sii902x(bridge);
>> +
>> + return sii902x_detect(sii902x);
>> +}
>> +
>> +static struct edid *sii902x_bridge_get_edid(struct drm_bridge *bridge,
>> + struct drm_connector *connector)
>> +{
>> + struct sii902x *sii902x = bridge_to_sii902x(bridge);
>> +
>> + return sii902x_get_edid(sii902x, connector);
>> +}
>> +
>> static const struct drm_bridge_funcs sii902x_bridge_funcs = {
>> .attach = sii902x_bridge_attach,
>> .mode_set = sii902x_bridge_mode_set,
>> .disable = sii902x_bridge_disable,
>> .enable = sii902x_bridge_enable,
>> + .detect = sii902x_bridge_detect,
>> + .get_edid = sii902x_bridge_get_edid,
>> };
>>
>> static int sii902x_mute(struct sii902x *sii902x, bool mute)
>> @@ -829,8 +866,12 @@ static irqreturn_t sii902x_interrupt(int irq, void *data)
>>
>> mutex_unlock(&sii902x->mutex);
>>
>> - if ((status & SII902X_HOTPLUG_EVENT) && sii902x->bridge.dev)
>> + if ((status & SII902X_HOTPLUG_EVENT) && sii902x->bridge.dev) {
>> drm_helper_hpd_irq_event(sii902x->bridge.dev);
>> + drm_bridge_hpd_notify(&sii902x->bridge, (status & SII902X_PLUGGED_STATUS)
>> + ? connector_status_connected
>> + : connector_status_disconnected);
>> + }
>>
>> return IRQ_HANDLED;
>> }
>> @@ -1001,6 +1042,11 @@ static int sii902x_init(struct sii902x *sii902x)
>> sii902x->bridge.funcs = &sii902x_bridge_funcs;
>> sii902x->bridge.of_node = dev->of_node;
>> sii902x->bridge.timings = &default_sii902x_timings;
>> + sii902x->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
>> +
>> + if (sii902x->i2c->irq > 0)
>> + sii902x->bridge.ops |= DRM_BRIDGE_OP_HPD;
>> +
>> drm_bridge_add(&sii902x->bridge);
>>
>> sii902x_audio_codec_init(sii902x, dev);
>> @@ -1022,6 +1068,7 @@ static int sii902x_probe(struct i2c_client *client,
>> const struct i2c_device_id *id)
>> {
>> struct device *dev = &client->dev;
>> + struct device_node *endpoint;
>> struct sii902x *sii902x;
>> int ret;
>>
>> @@ -1049,6 +1096,28 @@ static int sii902x_probe(struct i2c_client *client,
>> return PTR_ERR(sii902x->reset_gpio);
>> }
>>
>> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
>> + if (endpoint) {
>> + struct device_node *remote = of_graph_get_remote_port_parent(endpoint);
>> +
>> + of_node_put(endpoint);
>> + if (!remote) {
>> + dev_err(dev, "Endpoint in port@1 unconnected\n");
>> + return -ENODEV;
>> + }
>> +
>> + if (!of_device_is_available(remote)) {
>> + dev_err(dev, "port@1 remote device is disabled\n");
>> + of_node_put(remote);
>> + return -ENODEV;
>> + }
>> +
>> + sii902x->next_bridge = of_drm_find_bridge(remote);
>> + of_node_put(remote);
>> + if (!sii902x->next_bridge)
>> + return -EPROBE_DEFER;
>
> Hi,
>
> This patch broke ARM/QEMU vexpress display because of_drm_find_bridge()
> always fail with -EPROBE_DEFER. Reverting this patch returns display
> back. Please fix or revert, thanks in advance.
Can you share a QEMU cmdline to reproduce ?
Neil
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR
2022-08-08 9:15 ` Neil Armstrong
@ 2022-08-08 9:51 ` Neil Armstrong
2022-08-15 0:15 ` Dmitry Osipenko
2022-08-15 0:18 ` Dmitry Osipenko
1 sibling, 1 reply; 11+ messages in thread
From: Neil Armstrong @ 2022-08-08 9:51 UTC (permalink / raw)
To: Dmitry Osipenko, andrzej.hajda, robert.foss, Linus Walleij, Rob Herring
Cc: jernej.skrabec, jonas, linux-kernel, dri-devel, Laurent.pinchart
On 08/08/2022 11:15, Neil Armstrong wrote:
> Hi Dmitry,
>
> On 31/07/2022 22:07, Dmitry Osipenko wrote:
>> 13.01.2022 17:43, Neil Armstrong пишет:
>>> This adds support for DRM_BRIDGE_ATTACH_NO_CONNECTOR by adding the
>>> bridge get_edid() and detect() callbacks after refactoring the connector
>>> get_modes() and connector_detect() callbacks.
>>>
>>> In order to keep the bridge working, extra code in get_modes() has been
>>> moved to more logical places.
>>>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>> drivers/gpu/drm/bridge/sii902x.c | 129 ++++++++++++++++++++++++-------
>
> 1 file changed, 99 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
>>> index 89558e581530..65549fbfdc87 100644
>>> --- a/drivers/gpu/drm/bridge/sii902x.c
>>> +++ b/drivers/gpu/drm/bridge/sii902x.c
[...]
>>> }
>>> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
>>> + if (endpoint) {
>>> + struct device_node *remote = of_graph_get_remote_port_parent(endpoint);
>>> +
>>> + of_node_put(endpoint);
>>> + if (!remote) {
>>> + dev_err(dev, "Endpoint in port@1 unconnected\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + if (!of_device_is_available(remote)) {
>>> + dev_err(dev, "port@1 remote device is disabled\n");
>>> + of_node_put(remote);
>>> + return -ENODEV;
>>> + }
>>> +
>>> + sii902x->next_bridge = of_drm_find_bridge(remote);
>>> + of_node_put(remote);
>>> + if (!sii902x->next_bridge)
>>> + return -EPROBE_DEFER;
>>
>> Hi,
>>
>> This patch broke ARM/QEMU vexpress display because of_drm_find_bridge()
>> always fail with -EPROBE_DEFER. Reverting this patch returns display
>> back. Please fix or revert, thanks in advance.
>
> Can you share a QEMU cmdline to reproduce ?
Actually the vexpress DT has multiple input ports instead of a single input port at @0
and an output port at @1 like documented in the bindings:
vexpress-v2m.dtsi#L303-L307:
ports {
#address-cells = <1>;
#size-cells = <0>;
/*
* Both the core tile and the motherboard routes their output
* pads to this transmitter. The motherboard system controller
* can select one of them as input using a mux register in
* "arm,vexpress-muxfpga". The Vexpress with the CA9 core tile is
* the only platform with this specific set-up.
*/
port@0 {
reg = <0>;
dvi_bridge_in_ct: endpoint {
remote-endpoint = <&clcd_pads_ct>;
};
};
port@1 {
reg = <1>;
dvi_bridge_in_mb: endpoint {
remote-endpoint = <&clcd_pads_mb>;
};
};
};
bindings:
ports:
$ref: /schemas/graph.yaml#/properties/ports
properties:
port@0:
$ref: /schemas/graph.yaml#/properties/port
description: Parallel RGB input port
port@1:
$ref: /schemas/graph.yaml#/properties/port
description: HDMI output port
port@3:
$ref: /schemas/graph.yaml#/properties/port
description: Sound input port
The patch is conform to the bindings, the DT was working but is actually not valid.
Neil
>
> Neil
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR
2022-08-08 9:51 ` Neil Armstrong
@ 2022-08-15 0:15 ` Dmitry Osipenko
2022-08-17 13:31 ` Neil Armstrong
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Osipenko @ 2022-08-15 0:15 UTC (permalink / raw)
To: Neil Armstrong, andrzej.hajda, robert.foss, Linus Walleij, Rob Herring
Cc: jernej.skrabec, jonas, linux-kernel, dri-devel, Laurent.pinchart
08.08.2022 12:51, Neil Armstrong пишет:
> On 08/08/2022 11:15, Neil Armstrong wrote:
>> Hi Dmitry,
>>
>> On 31/07/2022 22:07, Dmitry Osipenko wrote:
>>> 13.01.2022 17:43, Neil Armstrong пишет:
>>>> This adds support for DRM_BRIDGE_ATTACH_NO_CONNECTOR by adding the
>>>> bridge get_edid() and detect() callbacks after refactoring the
>>>> connector
>>>> get_modes() and connector_detect() callbacks.
>>>>
>>>> In order to keep the bridge working, extra code in get_modes() has been
>>>> moved to more logical places.
>>>>
>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>> ---
>>>> drivers/gpu/drm/bridge/sii902x.c | 129
>>>> ++++++++++++++++++++++++-------
>>
>> 1 file changed, 99 insertions(+), 30 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/sii902x.c
>>>> b/drivers/gpu/drm/bridge/sii902x.c
>>>> index 89558e581530..65549fbfdc87 100644
>>>> --- a/drivers/gpu/drm/bridge/sii902x.c
>>>> +++ b/drivers/gpu/drm/bridge/sii902x.c
>
> [...]
>
>>>> }
>>>> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
>>>> + if (endpoint) {
>>>> + struct device_node *remote =
>>>> of_graph_get_remote_port_parent(endpoint);
>>>> +
>>>> + of_node_put(endpoint);
>>>> + if (!remote) {
>>>> + dev_err(dev, "Endpoint in port@1 unconnected\n");
>>>> + return -ENODEV;
>>>> + }
>>>> +
>>>> + if (!of_device_is_available(remote)) {
>>>> + dev_err(dev, "port@1 remote device is disabled\n");
>>>> + of_node_put(remote);
>>>> + return -ENODEV;
>>>> + }
>>>> +
>>>> + sii902x->next_bridge = of_drm_find_bridge(remote);
>>>> + of_node_put(remote);
>>>> + if (!sii902x->next_bridge)
>>>> + return -EPROBE_DEFER;
>>>
>>> Hi,
>>>
>>> This patch broke ARM/QEMU vexpress display because of_drm_find_bridge()
>>> always fail with -EPROBE_DEFER. Reverting this patch returns display
>>> back. Please fix or revert, thanks in advance.
>>
>> Can you share a QEMU cmdline to reproduce ?
>
> Actually the vexpress DT has multiple input ports instead of a single
> input port at @0
> and an output port at @1 like documented in the bindings:
>
> vexpress-v2m.dtsi#L303-L307:
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> /*
> * Both the core tile and the motherboard routes their output
> * pads to this transmitter. The motherboard system controller
> * can select one of them as input using a mux register in
> * "arm,vexpress-muxfpga". The Vexpress with the CA9 core tile is
> * the only platform with this specific set-up.
> */
> port@0 {
> reg = <0>;
> dvi_bridge_in_ct: endpoint {
> remote-endpoint = <&clcd_pads_ct>;
> };
> };
> port@1 {
> reg = <1>;
> dvi_bridge_in_mb: endpoint {
> remote-endpoint = <&clcd_pads_mb>;
> };
> };
> };
>
> bindings:
> ports:
> $ref: /schemas/graph.yaml#/properties/ports
>
> properties:
> port@0:
> $ref: /schemas/graph.yaml#/properties/port
> description: Parallel RGB input port
>
> port@1:
> $ref: /schemas/graph.yaml#/properties/port
> description: HDMI output port
>
> port@3:
> $ref: /schemas/graph.yaml#/properties/port
> description: Sound input port
>
> The patch is conform to the bindings, the DT was working but is actually
> not valid.
I haven't looked closely at how to fix this properly, but if we can fix
it using of_machine_is_compatible("arm,vexpress") workaround in the
driver, then it will be good enough at least as a temporal fix, IMO.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR
2022-08-08 9:15 ` Neil Armstrong
2022-08-08 9:51 ` Neil Armstrong
@ 2022-08-15 0:18 ` Dmitry Osipenko
1 sibling, 0 replies; 11+ messages in thread
From: Dmitry Osipenko @ 2022-08-15 0:18 UTC (permalink / raw)
To: Neil Armstrong, andrzej.hajda, robert.foss
Cc: jernej.skrabec, jonas, linux-kernel, dri-devel, Laurent.pinchart
08.08.2022 12:15, Neil Armstrong пишет:
> Hi Dmitry,
>
> On 31/07/2022 22:07, Dmitry Osipenko wrote:
>> 13.01.2022 17:43, Neil Armstrong пишет:
>>> This adds support for DRM_BRIDGE_ATTACH_NO_CONNECTOR by adding the
>>> bridge get_edid() and detect() callbacks after refactoring the connector
>>> get_modes() and connector_detect() callbacks.
>>>
>>> In order to keep the bridge working, extra code in get_modes() has been
>>> moved to more logical places.
>>>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>> ---
>>> drivers/gpu/drm/bridge/sii902x.c | 129 ++++++++++++++++++++++++-------
>>>
>
> 1 file changed, 99 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/sii902x.c
>>> b/drivers/gpu/drm/bridge/sii902x.c
>>> index 89558e581530..65549fbfdc87 100644
>>> --- a/drivers/gpu/drm/bridge/sii902x.c
>>> +++ b/drivers/gpu/drm/bridge/sii902x.c
>>> @@ -166,10 +166,12 @@ struct sii902x {
>>> struct i2c_client *i2c;
>>> struct regmap *regmap;
>>> struct drm_bridge bridge;
>>> + struct drm_bridge *next_bridge;
>>> struct drm_connector connector;
>>> struct gpio_desc *reset_gpio;
>>> struct i2c_mux_core *i2cmux;
>>> struct regulator_bulk_data supplies[2];
>>> + bool sink_is_hdmi;
>>> /*
>>> * Mutex protects audio and video functions from interfering
>>> * each other, by keeping their i2c command sequences atomic.
>>> @@ -245,10 +247,8 @@ static void sii902x_reset(struct sii902x *sii902x)
>>> gpiod_set_value(sii902x->reset_gpio, 0);
>>> }
>>> -static enum drm_connector_status
>>> -sii902x_connector_detect(struct drm_connector *connector, bool force)
>>> +static enum drm_connector_status sii902x_detect(struct sii902x
>>> *sii902x)
>>> {
>>> - struct sii902x *sii902x = connector_to_sii902x(connector);
>>> unsigned int status;
>>> mutex_lock(&sii902x->mutex);
>>> @@ -261,6 +261,14 @@ sii902x_connector_detect(struct drm_connector
>>> *connector, bool force)
>>> connector_status_connected : connector_status_disconnected;
>>> }
>>> +static enum drm_connector_status
>>> +sii902x_connector_detect(struct drm_connector *connector, bool force)
>>> +{
>>> + struct sii902x *sii902x = connector_to_sii902x(connector);
>>> +
>>> + return sii902x_detect(sii902x);
>>> +}
>>> +
>>> static const struct drm_connector_funcs sii902x_connector_funcs = {
>>> .detect = sii902x_connector_detect,
>>> .fill_modes = drm_helper_probe_single_connector_modes,
>>> @@ -270,42 +278,40 @@ static const struct drm_connector_funcs
>>> sii902x_connector_funcs = {
>>> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>>> };
>>> -static int sii902x_get_modes(struct drm_connector *connector)
>>> +static struct edid *sii902x_get_edid(struct sii902x *sii902x,
>>> + struct drm_connector *connector)
>>> {
>>> - struct sii902x *sii902x = connector_to_sii902x(connector);
>>> - u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
>>> - u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
>>> struct edid *edid;
>>> - int num = 0, ret;
>>> mutex_lock(&sii902x->mutex);
>>> edid = drm_get_edid(connector, sii902x->i2cmux->adapter[0]);
>>> - drm_connector_update_edid_property(connector, edid);
>>> if (edid) {
>>> if (drm_detect_hdmi_monitor(edid))
>>> - output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
>>> -
>>> - num = drm_add_edid_modes(connector, edid);
>>> - kfree(edid);
>>> + sii902x->sink_is_hdmi = true;
>>> + else
>>> + sii902x->sink_is_hdmi = false;
>>> }
>>> - ret = drm_display_info_set_bus_formats(&connector->display_info,
>>> - &bus_format, 1);
>>> - if (ret)
>>> - goto error_out;
>>> + mutex_unlock(&sii902x->mutex);
>>> - ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
>>> - SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
>>> - if (ret)
>>> - goto error_out;
>>> + return edid;
>>> +}
>>> - ret = num;
>>> +static int sii902x_get_modes(struct drm_connector *connector)
>>> +{
>>> + struct sii902x *sii902x = connector_to_sii902x(connector);
>>> + struct edid *edid;
>>> + int num = 0;
>>> -error_out:
>>> - mutex_unlock(&sii902x->mutex);
>>> + edid = sii902x_get_edid(sii902x, connector);
>>> + drm_connector_update_edid_property(connector, edid);
>>> + if (edid) {
>>> + num = drm_add_edid_modes(connector, edid);
>>> + kfree(edid);
>>> + }
>>> - return ret;
>>> + return num;
>>> }
>>> static enum drm_mode_status sii902x_mode_valid(struct
>>> drm_connector *connector,
>>> @@ -354,12 +360,16 @@ static void sii902x_bridge_mode_set(struct
>>> drm_bridge *bridge,
>>> const struct drm_display_mode *adj)
>>> {
>>> struct sii902x *sii902x = bridge_to_sii902x(bridge);
>>> + u8 output_mode = SII902X_SYS_CTRL_OUTPUT_DVI;
>>> struct regmap *regmap = sii902x->regmap;
>>> u8 buf[HDMI_INFOFRAME_SIZE(AVI)];
>>> struct hdmi_avi_infoframe frame;
>>> u16 pixel_clock_10kHz = adj->clock / 10;
>>> int ret;
>>> + if (sii902x->sink_is_hdmi)
>>> + output_mode = SII902X_SYS_CTRL_OUTPUT_HDMI;
>>> +
>>> buf[0] = pixel_clock_10kHz & 0xff;
>>> buf[1] = pixel_clock_10kHz >> 8;
>>> buf[2] = drm_mode_vrefresh(adj);
>>> @@ -375,6 +385,11 @@ static void sii902x_bridge_mode_set(struct
>>> drm_bridge *bridge,
>>> mutex_lock(&sii902x->mutex);
>>> + ret = regmap_update_bits(sii902x->regmap, SII902X_SYS_CTRL_DATA,
>>> + SII902X_SYS_CTRL_OUTPUT_MODE, output_mode);
>>> + if (ret)
>>> + goto out;
>>> +
>>> ret = regmap_bulk_write(regmap, SII902X_TPI_VIDEO_DATA, buf, 10);
>>> if (ret)
>>> goto out;
>>> @@ -405,13 +420,13 @@ static int sii902x_bridge_attach(struct
>>> drm_bridge *bridge,
>>> enum drm_bridge_attach_flags flags)
>>> {
>>> struct sii902x *sii902x = bridge_to_sii902x(bridge);
>>> + u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
>>> struct drm_device *drm = bridge->dev;
>>> int ret;
>>> - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
>>> - DRM_ERROR("Fix bridge driver to make connector optional!");
>>> - return -EINVAL;
>>> - }
>>> + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)
>>> + return drm_bridge_attach(bridge->encoder, sii902x->next_bridge,
>>> + bridge, flags);
>>> drm_connector_helper_add(&sii902x->connector,
>>> &sii902x_connector_helper_funcs);
>>> @@ -433,16 +448,38 @@ static int sii902x_bridge_attach(struct
>>> drm_bridge *bridge,
>>> else
>>> sii902x->connector.polled = DRM_CONNECTOR_POLL_CONNECT;
>>> + ret =
>>> drm_display_info_set_bus_formats(&sii902x->connector.display_info,
>>> + &bus_format, 1);
>>> + if (ret)
>>> + return ret;
>>> +
>>> drm_connector_attach_encoder(&sii902x->connector,
>>> bridge->encoder);
>>> return 0;
>>> }
>>> +static enum drm_connector_status sii902x_bridge_detect(struct
>>> drm_bridge *bridge)
>>> +{
>>> + struct sii902x *sii902x = bridge_to_sii902x(bridge);
>>> +
>>> + return sii902x_detect(sii902x);
>>> +}
>>> +
>>> +static struct edid *sii902x_bridge_get_edid(struct drm_bridge *bridge,
>>> + struct drm_connector *connector)
>>> +{
>>> + struct sii902x *sii902x = bridge_to_sii902x(bridge);
>>> +
>>> + return sii902x_get_edid(sii902x, connector);
>>> +}
>>> +
>>> static const struct drm_bridge_funcs sii902x_bridge_funcs = {
>>> .attach = sii902x_bridge_attach,
>>> .mode_set = sii902x_bridge_mode_set,
>>> .disable = sii902x_bridge_disable,
>>> .enable = sii902x_bridge_enable,
>>> + .detect = sii902x_bridge_detect,
>>> + .get_edid = sii902x_bridge_get_edid,
>>> };
>>> static int sii902x_mute(struct sii902x *sii902x, bool mute)
>>> @@ -829,8 +866,12 @@ static irqreturn_t sii902x_interrupt(int irq,
>>> void *data)
>>> mutex_unlock(&sii902x->mutex);
>>> - if ((status & SII902X_HOTPLUG_EVENT) && sii902x->bridge.dev)
>>> + if ((status & SII902X_HOTPLUG_EVENT) && sii902x->bridge.dev) {
>>> drm_helper_hpd_irq_event(sii902x->bridge.dev);
>>> + drm_bridge_hpd_notify(&sii902x->bridge, (status &
>>> SII902X_PLUGGED_STATUS)
>>> + ? connector_status_connected
>>> + : connector_status_disconnected);
>>> + }
>>> return IRQ_HANDLED;
>>> }
>>> @@ -1001,6 +1042,11 @@ static int sii902x_init(struct sii902x *sii902x)
>>> sii902x->bridge.funcs = &sii902x_bridge_funcs;
>>> sii902x->bridge.of_node = dev->of_node;
>>> sii902x->bridge.timings = &default_sii902x_timings;
>>> + sii902x->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
>>> +
>>> + if (sii902x->i2c->irq > 0)
>>> + sii902x->bridge.ops |= DRM_BRIDGE_OP_HPD;
>>> +
>>> drm_bridge_add(&sii902x->bridge);
>>> sii902x_audio_codec_init(sii902x, dev);
>>> @@ -1022,6 +1068,7 @@ static int sii902x_probe(struct i2c_client
>>> *client,
>>> const struct i2c_device_id *id)
>>> {
>>> struct device *dev = &client->dev;
>>> + struct device_node *endpoint;
>>> struct sii902x *sii902x;
>>> int ret;
>>> @@ -1049,6 +1096,28 @@ static int sii902x_probe(struct i2c_client
>>> *client,
>>> return PTR_ERR(sii902x->reset_gpio);
>>> }
>>> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
>>> + if (endpoint) {
>>> + struct device_node *remote =
>>> of_graph_get_remote_port_parent(endpoint);
>>> +
>>> + of_node_put(endpoint);
>>> + if (!remote) {
>>> + dev_err(dev, "Endpoint in port@1 unconnected\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + if (!of_device_is_available(remote)) {
>>> + dev_err(dev, "port@1 remote device is disabled\n");
>>> + of_node_put(remote);
>>> + return -ENODEV;
>>> + }
>>> +
>>> + sii902x->next_bridge = of_drm_find_bridge(remote);
>>> + of_node_put(remote);
>>> + if (!sii902x->next_bridge)
>>> + return -EPROBE_DEFER;
>>
>> Hi,
>>
>> This patch broke ARM/QEMU vexpress display because of_drm_find_bridge()
>> always fail with -EPROBE_DEFER. Reverting this patch returns display
>> back. Please fix or revert, thanks in advance.
>
> Can you share a QEMU cmdline to reproduce ?
qemu-system-arm -cpu cortex-a9 -M vexpress-a9 -smp 2 -m 1024 -kernel
arch/arm/boot/zImage -dtb arch/arm/boot/dts/vexpress-v2p-ca9.dtb -serial
stdio -net nic,model=lan9118 -net user
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR
2022-08-15 0:15 ` Dmitry Osipenko
@ 2022-08-17 13:31 ` Neil Armstrong
2022-08-25 12:48 ` Linus Walleij
0 siblings, 1 reply; 11+ messages in thread
From: Neil Armstrong @ 2022-08-17 13:31 UTC (permalink / raw)
To: Dmitry Osipenko, andrzej.hajda, robert.foss, Linus Walleij, Rob Herring
Cc: jernej.skrabec, jonas, linux-kernel, dri-devel, Laurent.pinchart
On 15/08/2022 02:15, Dmitry Osipenko wrote:
> 08.08.2022 12:51, Neil Armstrong пишет:
>> On 08/08/2022 11:15, Neil Armstrong wrote:
>>> Hi Dmitry,
>>>
>>> On 31/07/2022 22:07, Dmitry Osipenko wrote:
>>>> 13.01.2022 17:43, Neil Armstrong пишет:
>>>>> This adds support for DRM_BRIDGE_ATTACH_NO_CONNECTOR by adding the
>>>>> bridge get_edid() and detect() callbacks after refactoring the
>>>>> connector
>>>>> get_modes() and connector_detect() callbacks.
>>>>>
>>>>> In order to keep the bridge working, extra code in get_modes() has been
>>>>> moved to more logical places.
>>>>>
>>>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>>>> ---
>>>>> drivers/gpu/drm/bridge/sii902x.c | 129
>>>>> ++++++++++++++++++++++++-------
>>>
>>> 1 file changed, 99 insertions(+), 30 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/bridge/sii902x.c
>>>>> b/drivers/gpu/drm/bridge/sii902x.c
>>>>> index 89558e581530..65549fbfdc87 100644
>>>>> --- a/drivers/gpu/drm/bridge/sii902x.c
>>>>> +++ b/drivers/gpu/drm/bridge/sii902x.c
>>
>> [...]
>>
>>>>> }
>>>>> + endpoint = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
>>>>> + if (endpoint) {
>>>>> + struct device_node *remote =
>>>>> of_graph_get_remote_port_parent(endpoint);
>>>>> +
>>>>> + of_node_put(endpoint);
>>>>> + if (!remote) {
>>>>> + dev_err(dev, "Endpoint in port@1 unconnected\n");
>>>>> + return -ENODEV;
>>>>> + }
>>>>> +
>>>>> + if (!of_device_is_available(remote)) {
>>>>> + dev_err(dev, "port@1 remote device is disabled\n");
>>>>> + of_node_put(remote);
>>>>> + return -ENODEV;
>>>>> + }
>>>>> +
>>>>> + sii902x->next_bridge = of_drm_find_bridge(remote);
>>>>> + of_node_put(remote);
>>>>> + if (!sii902x->next_bridge)
>>>>> + return -EPROBE_DEFER;
>>>>
>>>> Hi,
>>>>
>>>> This patch broke ARM/QEMU vexpress display because of_drm_find_bridge()
>>>> always fail with -EPROBE_DEFER. Reverting this patch returns display
>>>> back. Please fix or revert, thanks in advance.
>>>
>>> Can you share a QEMU cmdline to reproduce ?
>>
>> Actually the vexpress DT has multiple input ports instead of a single
>> input port at @0
>> and an output port at @1 like documented in the bindings:
>>
>> vexpress-v2m.dtsi#L303-L307:
>> ports {
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> /*
>> * Both the core tile and the motherboard routes their output
>> * pads to this transmitter. The motherboard system controller
>> * can select one of them as input using a mux register in
>> * "arm,vexpress-muxfpga". The Vexpress with the CA9 core tile is
>> * the only platform with this specific set-up.
>> */
>> port@0 {
>> reg = <0>;
>> dvi_bridge_in_ct: endpoint {
>> remote-endpoint = <&clcd_pads_ct>;
>> };
>> };
>> port@1 {
>> reg = <1>;
>> dvi_bridge_in_mb: endpoint {
>> remote-endpoint = <&clcd_pads_mb>;
>> };
>> };
>> };
>>
>> bindings:
>> ports:
>> $ref: /schemas/graph.yaml#/properties/ports
>>
>> properties:
>> port@0:
>> $ref: /schemas/graph.yaml#/properties/port
>> description: Parallel RGB input port
>>
>> port@1:
>> $ref: /schemas/graph.yaml#/properties/port
>> description: HDMI output port
>>
>> port@3:
>> $ref: /schemas/graph.yaml#/properties/port
>> description: Sound input port
>>
>> The patch is conform to the bindings, the DT was working but is actually
>> not valid.
>
> I haven't looked closely at how to fix this properly, but if we can fix
> it using of_machine_is_compatible("arm,vexpress") workaround in the
> driver, then it will be good enough at least as a temporal fix, IMO.
If other maintainers are ok with that, it can be temporary fix until the DT gets fixed.
Neil
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR
2022-08-17 13:31 ` Neil Armstrong
@ 2022-08-25 12:48 ` Linus Walleij
2022-08-29 13:36 ` Neil Armstrong
0 siblings, 1 reply; 11+ messages in thread
From: Linus Walleij @ 2022-08-25 12:48 UTC (permalink / raw)
To: Neil Armstrong
Cc: Dmitry Osipenko, andrzej.hajda, robert.foss, Rob Herring,
jernej.skrabec, jonas, linux-kernel, dri-devel, Laurent.pinchart,
Sudeep Holla
On Wed, Aug 17, 2022 at 3:31 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> On 15/08/2022 02:15, Dmitry Osipenko wrote:
> > 08.08.2022 12:51, Neil Armstrong пишет:
> >> On 08/08/2022 11:15, Neil Armstrong wrote:
> >>>> This patch broke ARM/QEMU vexpress display because of_drm_find_bridge()
> >>>> always fail with -EPROBE_DEFER. Reverting this patch returns display
> >>>> back. Please fix or revert, thanks in advance.
> >>>
> >>> Can you share a QEMU cmdline to reproduce ?
> >>
> >> Actually the vexpress DT has multiple input ports instead of a single
> >> input port at @0
> >> and an output port at @1 like documented in the bindings:
> >>
> >> vexpress-v2m.dtsi#L303-L307:
> >> ports {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >>
> >> /*
> >> * Both the core tile and the motherboard routes their output
> >> * pads to this transmitter. The motherboard system controller
> >> * can select one of them as input using a mux register in
> >> * "arm,vexpress-muxfpga". The Vexpress with the CA9 core tile is
> >> * the only platform with this specific set-up.
> >> */
> >> port@0 {
> >> reg = <0>;
> >> dvi_bridge_in_ct: endpoint {
> >> remote-endpoint = <&clcd_pads_ct>;
> >> };
> >> };
> >> port@1 {
> >> reg = <1>;
> >> dvi_bridge_in_mb: endpoint {
> >> remote-endpoint = <&clcd_pads_mb>;
> >> };
> >> };
> >> };
> >>
> >> bindings:
> >> ports:
> >> $ref: /schemas/graph.yaml#/properties/ports
> >>
> >> properties:
> >> port@0:
> >> $ref: /schemas/graph.yaml#/properties/port
> >> description: Parallel RGB input port
> >>
> >> port@1:
> >> $ref: /schemas/graph.yaml#/properties/port
> >> description: HDMI output port
> >>
> >> port@3:
> >> $ref: /schemas/graph.yaml#/properties/port
> >> description: Sound input port
> >>
> >> The patch is conform to the bindings, the DT was working but is actually
> >> not valid.
> >
> > I haven't looked closely at how to fix this properly, but if we can fix
> > it using of_machine_is_compatible("arm,vexpress") workaround in the
> > driver, then it will be good enough at least as a temporal fix, IMO.
>
> If other maintainers are ok with that, it can be temporary fix until the DT gets fixed.
That's fine with me, will you send a patch?
I don't know how you expect the DT to get "fixed" though.
The hardware looks like this, it's maybe not the most elegant
electronics design but it exists, so... I wrote this DT with two
inputs, see commit f1fe12c8bf332, the code handling this
awkward mux is part of the DRM driver, see
drivers/gpu/drm/pl111/pl111_versatile.c function
pl111_vexpress_clcd_init() for an idea of how it works.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR
2022-08-25 12:48 ` Linus Walleij
@ 2022-08-29 13:36 ` Neil Armstrong
2022-08-31 12:34 ` Linus Walleij
0 siblings, 1 reply; 11+ messages in thread
From: Neil Armstrong @ 2022-08-29 13:36 UTC (permalink / raw)
To: Linus Walleij
Cc: Dmitry Osipenko, andrzej.hajda, robert.foss, Rob Herring,
jernej.skrabec, jonas, linux-kernel, dri-devel, Laurent.pinchart,
Sudeep Holla
On 25/08/2022 14:48, Linus Walleij wrote:
> On Wed, Aug 17, 2022 at 3:31 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>> On 15/08/2022 02:15, Dmitry Osipenko wrote:
>>> 08.08.2022 12:51, Neil Armstrong пишет:
>>>> On 08/08/2022 11:15, Neil Armstrong wrote:
>
>>>>>> This patch broke ARM/QEMU vexpress display because of_drm_find_bridge()
>>>>>> always fail with -EPROBE_DEFER. Reverting this patch returns display
>>>>>> back. Please fix or revert, thanks in advance.
>>>>>
>>>>> Can you share a QEMU cmdline to reproduce ?
>>>>
>>>> Actually the vexpress DT has multiple input ports instead of a single
>>>> input port at @0
>>>> and an output port at @1 like documented in the bindings:
>>>>
>>>> vexpress-v2m.dtsi#L303-L307:
>>>> ports {
>>>> #address-cells = <1>;
>>>> #size-cells = <0>;
>>>>
>>>> /*
>>>> * Both the core tile and the motherboard routes their output
>>>> * pads to this transmitter. The motherboard system controller
>>>> * can select one of them as input using a mux register in
>>>> * "arm,vexpress-muxfpga". The Vexpress with the CA9 core tile is
>>>> * the only platform with this specific set-up.
>>>> */
>>>> port@0 {
>>>> reg = <0>;
>>>> dvi_bridge_in_ct: endpoint {
>>>> remote-endpoint = <&clcd_pads_ct>;
>>>> };
>>>> };
>>>> port@1 {
>>>> reg = <1>;
>>>> dvi_bridge_in_mb: endpoint {
>>>> remote-endpoint = <&clcd_pads_mb>;
>>>> };
>>>> };
>>>> };
>>>>
>>>> bindings:
>>>> ports:
>>>> $ref: /schemas/graph.yaml#/properties/ports
>>>>
>>>> properties:
>>>> port@0:
>>>> $ref: /schemas/graph.yaml#/properties/port
>>>> description: Parallel RGB input port
>>>>
>>>> port@1:
>>>> $ref: /schemas/graph.yaml#/properties/port
>>>> description: HDMI output port
>>>>
>>>> port@3:
>>>> $ref: /schemas/graph.yaml#/properties/port
>>>> description: Sound input port
>>>>
>>>> The patch is conform to the bindings, the DT was working but is actually
>>>> not valid.
>>>
>>> I haven't looked closely at how to fix this properly, but if we can fix
>>> it using of_machine_is_compatible("arm,vexpress") workaround in the
>>> driver, then it will be good enough at least as a temporal fix, IMO.
>>
>> If other maintainers are ok with that, it can be temporary fix until the DT gets fixed.
>
> That's fine with me, will you send a patch?
Who, me ?
>
> I don't know how you expect the DT to get "fixed" though.
>
> The hardware looks like this, it's maybe not the most elegant
> electronics design but it exists, so... I wrote this DT with two
> inputs, see commit f1fe12c8bf332, the code handling this
> awkward mux is part of the DRM driver, see
> drivers/gpu/drm/pl111/pl111_versatile.c function
> pl111_vexpress_clcd_init() for an idea of how it works.
The proper fix would be the other way around, adding a mux bridge before the sii902x
returning the next bridge or nothing to the right controller.
>
> Yours,
> Linus Walleij
Neil
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR
2022-08-29 13:36 ` Neil Armstrong
@ 2022-08-31 12:34 ` Linus Walleij
0 siblings, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2022-08-31 12:34 UTC (permalink / raw)
To: Neil Armstrong
Cc: Dmitry Osipenko, andrzej.hajda, robert.foss, Rob Herring,
jernej.skrabec, jonas, linux-kernel, dri-devel, Laurent.pinchart,
Sudeep Holla
On Mon, Aug 29, 2022 at 3:36 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> On 25/08/2022 14:48, Linus Walleij wrote:
> > On Wed, Aug 17, 2022 at 3:31 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >> On 15/08/2022 02:15, Dmitry Osipenko wrote:
> >>> 08.08.2022 12:51, Neil Armstrong пишет:
> >>>> On 08/08/2022 11:15, Neil Armstrong wrote:
> >
> >>>>>> This patch broke ARM/QEMU vexpress display because of_drm_find_bridge()
> >>>>>> always fail with -EPROBE_DEFER. Reverting this patch returns display
> >>>>>> back. Please fix or revert, thanks in advance.
> >>>>>
> >>>>> Can you share a QEMU cmdline to reproduce ?
> >>>>
> >>>> Actually the vexpress DT has multiple input ports instead of a single
> >>>> input port at @0
> >>>> and an output port at @1 like documented in the bindings:
> >>>>
> >>>> vexpress-v2m.dtsi#L303-L307:
> >>>> ports {
> >>>> #address-cells = <1>;
> >>>> #size-cells = <0>;
> >>>>
> >>>> /*
> >>>> * Both the core tile and the motherboard routes their output
> >>>> * pads to this transmitter. The motherboard system controller
> >>>> * can select one of them as input using a mux register in
> >>>> * "arm,vexpress-muxfpga". The Vexpress with the CA9 core tile is
> >>>> * the only platform with this specific set-up.
> >>>> */
> >>>> port@0 {
> >>>> reg = <0>;
> >>>> dvi_bridge_in_ct: endpoint {
> >>>> remote-endpoint = <&clcd_pads_ct>;
> >>>> };
> >>>> };
> >>>> port@1 {
> >>>> reg = <1>;
> >>>> dvi_bridge_in_mb: endpoint {
> >>>> remote-endpoint = <&clcd_pads_mb>;
> >>>> };
> >>>> };
> >>>> };
> >>>>
> >>>> bindings:
> >>>> ports:
> >>>> $ref: /schemas/graph.yaml#/properties/ports
> >>>>
> >>>> properties:
> >>>> port@0:
> >>>> $ref: /schemas/graph.yaml#/properties/port
> >>>> description: Parallel RGB input port
> >>>>
> >>>> port@1:
> >>>> $ref: /schemas/graph.yaml#/properties/port
> >>>> description: HDMI output port
> >>>>
> >>>> port@3:
> >>>> $ref: /schemas/graph.yaml#/properties/port
> >>>> description: Sound input port
> >>>>
> >>>> The patch is conform to the bindings, the DT was working but is actually
> >>>> not valid.
> >>>
> >>> I haven't looked closely at how to fix this properly, but if we can fix
> >>> it using of_machine_is_compatible("arm,vexpress") workaround in the
> >>> driver, then it will be good enough at least as a temporal fix, IMO.
> >>
> >> If other maintainers are ok with that, it can be temporary fix until the DT gets fixed.
> >
> > That's fine with me, will you send a patch?
>
> Who, me ?
Whoever you were referring to with the "temporary fix" :D
> > I don't know how you expect the DT to get "fixed" though.
> >
> > The hardware looks like this, it's maybe not the most elegant
> > electronics design but it exists, so... I wrote this DT with two
> > inputs, see commit f1fe12c8bf332, the code handling this
> > awkward mux is part of the DRM driver, see
> > drivers/gpu/drm/pl111/pl111_versatile.c function
> > pl111_vexpress_clcd_init() for an idea of how it works.
>
> The proper fix would be the other way around, adding a mux bridge before the sii902x
> returning the next bridge or nothing to the right controller.
Hm you mean the vexpress PLD should be a bridge and not just
some magic registers poked by the DRM driver?
OK fair point. But it will need proper representing in the DT then,
I guess that is what you mean by "DT gets fixed". It's not just
the DT that needs fixing then but also the driver(s).
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-08-31 12:36 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-13 14:43 [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR Neil Armstrong
2022-01-17 9:53 ` Robert Foss
2022-07-31 20:07 ` Dmitry Osipenko
2022-08-08 9:15 ` Neil Armstrong
2022-08-08 9:51 ` Neil Armstrong
2022-08-15 0:15 ` Dmitry Osipenko
2022-08-17 13:31 ` Neil Armstrong
2022-08-25 12:48 ` Linus Walleij
2022-08-29 13:36 ` Neil Armstrong
2022-08-31 12:34 ` Linus Walleij
2022-08-15 0:18 ` Dmitry Osipenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).