Messages in this thread | | | Date | Mon, 8 Aug 2022 11:15:42 +0200 | Subject | Re: [PATCH] drm/bridge: sii902x: add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR | From | 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 ?
Neil
| |